From ef67597f33827507acc0f92b6897b1eb84ae2dda Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Mon, 17 Jun 2024 16:47:57 -0400 Subject: [PATCH] [RHOAIENG-7942]: Update Side Panel Behavior to Pan Graph on Node Selection Fix side panel sizing, scrolling, and contents (#8) apply light variants to page section, fix linting issue Gage PR feedback fix tests --- .../pipeline/PipelineDetails.tsx | 312 +++++++++--------- .../pipeline/SelectedTaskDrawerContent.tsx | 7 +- .../pipelineRun/PipelineRunDetails.tsx | 162 +++++---- .../pipelineRun/PipelineRunDetailsTabs.tsx | 39 ++- .../PipelineRunDrawerRightContent.tsx | 4 +- .../pipelineRunJob/PipelineRunJobDetails.tsx | 127 ++++--- .../concepts/topology/PipelineTopology.tsx | 4 +- .../topology/PipelineVisualizationSurface.tsx | 33 ++ 8 files changed, 341 insertions(+), 347 deletions(-) diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx index 915fcdec68..e7f6ab0561 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx @@ -3,11 +3,9 @@ import { useNavigate, useParams } from 'react-router-dom'; import { Breadcrumb, BreadcrumbItem, - Drawer, - DrawerContent, - DrawerContentBody, Flex, FlexItem, + PageSection, Tab, TabContent, TabContentBody, @@ -84,171 +82,157 @@ const PipelineDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath }) = return ( <> - - setSelectedId(null)} - /> - } - > - - - {breadcrumbPath()} - - {/* TODO: Remove the custom className after upgrading to PFv6 */} - - - - {/* TODO: Remove the custom className after upgrading to PFv6 */} - - - - } - title={ - + {breadcrumbPath()} + + {/* TODO: Remove the custom className after upgrading to PFv6 */} + + + + {/* TODO: Remove the custom className after upgrading to PFv6 */} + + + + } + title={ + + } + {...(pipelineVersion && { + description: ( + + ), + })} + empty={false} + loaded={isLoaded} + headerAction={ + isPipelineVersionLoaded && ( + + + + navigate( + routePipelineDetailsNamespace( + namespace, + version.pipeline_id, + version.pipeline_version_id, + ), + ) + } /> - } - {...(pipelineVersion && { - description: ( - + + {isLoaded && ( + setDeletionOpen(true)} + pipeline={pipeline} + pipelineVersion={pipelineVersion} /> - ), - })} - empty={false} - loaded={isLoaded} - headerAction={ - isPipelineVersionLoaded && ( - - - - navigate( - routePipelineDetailsNamespace( - namespace, - version.pipeline_id, - version.pipeline_version_id, - ), - ) - } - /> - - - {isLoaded && ( - setDeletionOpen(true)} - pipeline={pipeline} - pipelineVersion={pipelineVersion} - /> - )} - - - ) - } + )} + + + ) + } + > + + { + setActiveTabKey(tabIndex); + setSelectedId(null); + }} + aria-label="Pipeline Details tabs" + role="region" + > + Graph} + aria-label="Pipeline Graph Tab" + tabContentId={`tabContent-${PipelineDetailsTab.GRAPH}`} + /> + Summary} + aria-label="Pipeline Summary Tab" > - { - setActiveTabKey(tabIndex); - setSelectedId(null); + + + + + Pipeline spec} + data-testid="pipeline-yaml-tab" + aria-label="Pipeline YAML Tab" + tabContentId={`tabContent-${PipelineDetailsTab.YAML}`} + /> + + - - - + sidePanel={ + setSelectedId(null)} + /> + } + /> + )} + + + + {pipeline && ( = ({ t } return ( - + {task.name} {task.type === 'artifact' ? 'Artifact details' : ''} diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx index ebd7bb8098..6cb2b98be4 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx @@ -2,8 +2,6 @@ import * as React from 'react'; import { Breadcrumb, BreadcrumbItem, - Drawer, - DrawerContent, EmptyState, EmptyStateIcon, EmptyStateVariant, @@ -96,99 +94,87 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath, const runType = run?.storage_state === StorageStateKF.ARCHIVED ? PipelineRunType.ARCHIVED : undefined; + const panelContent = selectedNode ? ( + <PipelineRunDrawerRightContent + task={selectedNode.data.pipelineTask} + upstreamTaskName={selectedNode.runAfterTasks?.[0]} + onClose={() => setSelectedId(null)} + executions={executions} + /> + ) : null; + return ( <> - <Drawer isExpanded={!!selectedNode}> - <DrawerContent - panelContent={ - <PipelineRunDrawerRightContent - task={selectedNode?.data.pipelineTask} - upstreamTaskName={selectedNode?.runAfterTasks?.[0]} - onClose={() => setSelectedId(null)} - executions={executions} + <ApplicationsPage + title={ + run ? <PipelineDetailsTitle run={run} statusIcon pipelineRunLabel /> : 'Error loading run' + } + subtext={ + run && ( + <PipelineJobReferenceName + runName={run.display_name} + recurringRunId={run.recurring_run_id} /> - } - > - <ApplicationsPage - title={ - run ? ( - <PipelineDetailsTitle run={run} statusIcon pipelineRunLabel /> - ) : ( - 'Error loading run' - ) - } - subtext={ - run && ( - <PipelineJobReferenceName - runName={run.display_name} - recurringRunId={run.recurring_run_id} - /> - ) - } - description={ - run?.description ? <MarkdownView conciseDisplay markdown={run.description} /> : '' - } - loaded={loaded} - loadError={error} - breadcrumb={ - <Breadcrumb> - {breadcrumbPath(runType)} - <BreadcrumbItem isActive style={{ maxWidth: 300 }}> - {version ? ( - <Link - to={routePipelineVersionRunsNamespace( - namespace, - version.pipeline_id, - version.pipeline_version_id, - runType, - )} - > - {/* TODO: Remove the custom className after upgrading to PFv6 */} - <Truncate content={version.display_name} className="truncate-no-min-width" /> - </Link> - ) : ( - 'Loading...' + ) + } + description={ + run?.description ? <MarkdownView conciseDisplay markdown={run.description} /> : '' + } + loaded={loaded} + loadError={error} + breadcrumb={ + <Breadcrumb> + {breadcrumbPath(runType)} + <BreadcrumbItem isActive style={{ maxWidth: 300 }}> + {version ? ( + <Link + to={routePipelineVersionRunsNamespace( + namespace, + version.pipeline_id, + version.pipeline_version_id, + runType, )} - </BreadcrumbItem> - <BreadcrumbItem isActive style={{ maxWidth: 300 }}> + > {/* TODO: Remove the custom className after upgrading to PFv6 */} - <Truncate - content={run?.display_name ?? 'Loading...'} - className="truncate-no-min-width" - /> - </BreadcrumbItem> - </Breadcrumb> - } - headerAction={ - <PipelineRunDetailsActions - run={run} - onDelete={() => setDeleting(true)} - onArchive={() => setArchiving(true)} + <Truncate content={version.display_name} className="truncate-no-min-width" /> + </Link> + ) : ( + 'Loading...' + )} + </BreadcrumbItem> + <BreadcrumbItem isActive style={{ maxWidth: 300 }}> + {/* TODO: Remove the custom className after upgrading to PFv6 */} + <Truncate + content={run?.display_name ?? 'Loading...'} + className="truncate-no-min-width" /> - } - empty={false} - > - <PipelineRunDetailsTabs - run={run} - pipelineSpec={version?.pipeline_spec} - graphContent={ - <PipelineTopology - nodes={nodes} - selectedIds={selectedId ? [selectedId] : []} - onSelectionChange={(ids) => { - const firstId = ids[0]; - if (ids.length === 0) { - setSelectedId(null); - } else if (nodes.find((node) => node.id === firstId)) { - setSelectedId(firstId); - } - }} - /> - } + </BreadcrumbItem> + </Breadcrumb> + } + headerAction={ + <PipelineRunDetailsActions + run={run} + onDelete={() => setDeleting(true)} + onArchive={() => setArchiving(true)} + /> + } + empty={false} + > + <PipelineRunDetailsTabs + run={run} + pipelineSpec={version?.pipeline_spec} + graphContent={ + <PipelineTopology + nodes={nodes} + selectedIds={selectedId ? [selectedId] : []} + onSelectionChange={(ids) => { + setSelectedId(ids.length ? ids[0] : null); + }} + sidePanel={panelContent} /> - </ApplicationsPage> - </DrawerContent> - </Drawer> + } + /> + </ApplicationsPage> <DeletePipelineRunsModal type={PipelineRunType.ARCHIVED} toDeleteResources={deleting && run ? [run] : []} diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsTabs.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsTabs.tsx index 908cd4525a..604779d934 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsTabs.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsTabs.tsx @@ -1,6 +1,13 @@ import React from 'react'; -import { Tabs, Tab, TabTitleText, TabContentBody, TabContent } from '@patternfly/react-core'; +import { + Tabs, + Tab, + TabTitleText, + TabContentBody, + TabContent, + PageSection, +} from '@patternfly/react-core'; import PipelineDetailsYAML from '~/concepts/pipelines/content/pipelinesDetails/PipelineDetailsYAML'; import { @@ -32,7 +39,12 @@ export const PipelineRunDetailsTabs: React.FC<PipelineRunDetailsTabsProps> = ({ const isJob = run && isPipelineRunJob(run); return ( - <> + <PageSection + isFilled + padding={{ default: 'noPadding' }} + style={{ flexBasis: 0, overflowY: 'hidden' }} + variant="light" + > <Tabs activeKey={activeKey} onSelect={(_, eventKey) => setActiveKey(eventKey)} @@ -45,7 +57,6 @@ export const PipelineRunDetailsTabs: React.FC<PipelineRunDetailsTabsProps> = ({ aria-label="Run graph tab" data-testid="pipeline-run-tab-graph" /> - <Tab eventKey={DetailsTabKey.Details} title={<TabTitleText>Details</TabTitleText>} @@ -56,7 +67,6 @@ export const PipelineRunDetailsTabs: React.FC<PipelineRunDetailsTabsProps> = ({ <PipelineRunTabDetails workflowName={run?.display_name} run={run} /> </TabContentBody> </Tab> - {!isJob && pipelineSpec && ( <Tab eventKey={DetailsTabKey.Spec} @@ -68,26 +78,25 @@ export const PipelineRunDetailsTabs: React.FC<PipelineRunDetailsTabsProps> = ({ )} </Tabs> - <div style={{ flex: 1 }} hidden={activeKey !== DetailsTabKey.Graph}> - <TabContent - id={DetailsTabKey.Graph} - eventKey={DetailsTabKey.Graph} - className="pf-v5-u-h-100" - > - <TabContentBody className="pf-v5-u-h-100">{graphContent}</TabContentBody> - </TabContent> - </div> - + <TabContent + id={DetailsTabKey.Graph} + eventKey={DetailsTabKey.Graph} + className="pf-v5-u-h-100" + hidden={activeKey !== DetailsTabKey.Graph} + > + <TabContentBody className="pf-v5-u-h-100">{graphContent}</TabContentBody> + </TabContent> <TabContent id={DetailsTabKey.Spec} eventKey={DetailsTabKey.Spec} hidden={activeKey !== DetailsTabKey.Spec} + className="pf-v5-u-h-100" style={{ flex: 1 }} > <TabContentBody className="pf-v5-u-h-100" hasPadding> <PipelineDetailsYAML filename={run?.display_name} content={pipelineSpec} /> </TabContentBody> </TabContent> - </> + </PageSection> ); }; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx index cc9b916457..87fb500b60 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx @@ -33,10 +33,8 @@ const PipelineRunDrawerRightContent: React.FC<PipelineRunDrawerRightContentProps return ( <DrawerPanelContent - isResizable - widths={{ default: 'width_33', lg: 'width_50' }} - minSize="500px" data-testid="pipeline-run-drawer-right-content" + style={{ height: '100%', overflowY: 'auto' }} > {task.type === 'artifact' ? ( <ArtifactNodeDrawerContent diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRunJob/PipelineRunJobDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRunJob/PipelineRunJobDetails.tsx index abb5335da2..ee0a22b007 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRunJob/PipelineRunJobDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRunJob/PipelineRunJobDetails.tsx @@ -2,8 +2,6 @@ import * as React from 'react'; import { Breadcrumb, BreadcrumbItem, - Drawer, - DrawerContent, EmptyState, EmptyStateIcon, EmptyStateVariant, @@ -51,8 +49,6 @@ const PipelineRunJobDetails: PipelineCoreDetailsPageComponent = ({ [selectedId, nodes], ); - const getFirstNode = (firstId: string) => nodes.find((n) => n.id === firstId)?.data?.pipelineTask; - const loaded = versionLoaded && jobLoaded; const error = versionError || jobError; @@ -76,76 +72,67 @@ const PipelineRunJobDetails: PipelineCoreDetailsPageComponent = ({ ); } + const panelContent = selectedNode ? ( + <SelectedTaskDrawerContent + task={selectedNode.data.pipelineTask} + onClose={() => setSelectedId(null)} + /> + ) : null; + return ( <> - <Drawer isExpanded={!!selectedNode}> - <DrawerContent - panelContent={ - <SelectedTaskDrawerContent - task={selectedNode?.data.pipelineTask} - onClose={() => setSelectedId(null)} - /> - } - > - <ApplicationsPage - title={job?.display_name} - description={job ? <MarkdownView conciseDisplay markdown={job.description} /> : ''} - loaded={loaded} - loadError={error} - breadcrumb={ - <Breadcrumb> - {breadcrumbPath(PipelineRunType.SCHEDULED)} - <BreadcrumbItem isActive style={{ maxWidth: 300 }}> - {version ? ( - <Link - to={routePipelineVersionRunsNamespace( - namespace, - version.pipeline_id, - version.pipeline_version_id, - PipelineRunType.SCHEDULED, - )} - > - {version.display_name} - </Link> - ) : ( - 'Loading...' + <ApplicationsPage + title={job?.display_name} + description={job ? <MarkdownView conciseDisplay markdown={job.description} /> : ''} + loaded={loaded} + loadError={error} + breadcrumb={ + <Breadcrumb> + {breadcrumbPath(PipelineRunType.SCHEDULED)} + <BreadcrumbItem isActive style={{ maxWidth: 300 }}> + {version ? ( + <Link + to={routePipelineVersionRunsNamespace( + namespace, + version.pipeline_id, + version.pipeline_version_id, + PipelineRunType.SCHEDULED, )} - </BreadcrumbItem> - <BreadcrumbItem isActive>{job?.display_name ?? 'Loading...'}</BreadcrumbItem> - </Breadcrumb> - } - headerAction={ - loaded && ( - <PipelineRunJobDetailsActions - job={job ?? undefined} - onDelete={() => setDeleting(true)} - /> - ) - } - empty={false} - > - <PipelineRunDetailsTabs - run={job} - pipelineSpec={version?.pipeline_spec} - graphContent={ - <PipelineTopology - nodes={nodes} - selectedIds={selectedId ? [selectedId] : []} - onSelectionChange={(ids) => { - const firstId = ids[0]; - if (ids.length === 0) { - setSelectedId(null); - } else if (getFirstNode(firstId)) { - setSelectedId(firstId); - } - }} - /> - } + > + {version.display_name} + </Link> + ) : ( + 'Loading...' + )} + </BreadcrumbItem> + <BreadcrumbItem isActive>{job?.display_name ?? 'Loading...'}</BreadcrumbItem> + </Breadcrumb> + } + headerAction={ + loaded && ( + <PipelineRunJobDetailsActions + job={job ?? undefined} + onDelete={() => setDeleting(true)} /> - </ApplicationsPage> - </DrawerContent> - </Drawer> - + ) + } + empty={false} + > + <PipelineRunDetailsTabs + run={job} + pipelineSpec={version?.pipeline_spec} + graphContent={ + <PipelineTopology + nodes={nodes} + selectedIds={selectedId ? [selectedId] : []} + onSelectionChange={(ids) => { + setSelectedId(ids.length ? ids[0] : null); + }} + sidePanel={panelContent} + /> + } + /> + </ApplicationsPage> <DeletePipelineRunsModal type={PipelineRunType.SCHEDULED} toDeleteResources={deleting && job ? [job] : []} diff --git a/frontend/src/concepts/topology/PipelineTopology.tsx b/frontend/src/concepts/topology/PipelineTopology.tsx index aba5f9e56f..2cd2807b97 100644 --- a/frontend/src/concepts/topology/PipelineTopology.tsx +++ b/frontend/src/concepts/topology/PipelineTopology.tsx @@ -13,12 +13,14 @@ type PipelineTopologyProps = { selectedIds?: string[]; onSelectionChange?: (selectionIds: string[]) => void; nodes: PipelineNodeModel[]; + sidePanel?: React.ReactElement | null; }; const PipelineTopology: React.FC<PipelineTopologyProps> = ({ nodes, selectedIds, onSelectionChange, + sidePanel, }) => { const controller = useTopologyController('g1'); @@ -51,7 +53,7 @@ const PipelineTopology: React.FC<PipelineTopologyProps> = ({ return ( <VisualizationProvider controller={controller}> - <PipelineVisualizationSurface nodes={nodes} selectedIds={selectedIds} /> + <PipelineVisualizationSurface nodes={nodes} selectedIds={selectedIds} sidePanel={sidePanel} /> </VisualizationProvider> ); }; diff --git a/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx b/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx index f05528ecbc..a338fb6c44 100644 --- a/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx +++ b/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx @@ -12,6 +12,7 @@ import { addSpacerNodes, DEFAULT_SPACER_NODE_TYPE, DEFAULT_EDGE_TYPE, + TopologySideBar, } from '@patternfly/react-topology'; import { EmptyState, @@ -25,14 +26,43 @@ import { NODE_HEIGHT, NODE_WIDTH } from './const'; type PipelineVisualizationSurfaceProps = { nodes: PipelineNodeModel[]; selectedIds?: string[]; + sidePanel?: React.ReactElement | null; }; const PipelineVisualizationSurface: React.FC<PipelineVisualizationSurfaceProps> = ({ nodes, selectedIds, + sidePanel, }) => { const controller = useVisualizationController(); const [error, setError] = React.useState<Error | null>(); + + const selectedNode = React.useMemo(() => { + if (selectedIds?.[0]) { + const node = controller.getNodeById(selectedIds[0]); + if (node) { + return node; + } + } + return null; + }, [selectedIds, controller]); + + React.useEffect(() => { + let resizeTimeout: NodeJS.Timeout | null; + if (selectedNode) { + // Use a timeout in order to allow the side panel to be shown and window size recomputed + resizeTimeout = setTimeout(() => { + controller.getGraph().panIntoView(selectedNode, { offset: 20, minimumVisible: 100 }); + resizeTimeout = null; + }, 500); + } + return () => { + if (resizeTimeout) { + clearTimeout(resizeTimeout); + } + }; + }, [selectedIds, controller, selectedNode]); + React.useEffect(() => { const currentModel = controller.toModel(); const updateNodes = nodes.map((node) => { @@ -158,6 +188,9 @@ const PipelineVisualizationSurface: React.FC<PipelineVisualizationSurfaceProps> })} /> } + sideBarOpen={!!selectedNode} + sideBarResizable + sideBar={<TopologySideBar resizable>{sidePanel}</TopologySideBar>} > <VisualizationSurface state={{ selectedIds }} /> </TopologyView>