From fb68429e6350a06d3b3a4a3f297c018e3e3f4568 Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Fri, 9 Oct 2026 09:49:39 -0400 Subject: [PATCH 01/17] feat(RHOAIENG-66823): make pipeline graphs accessible to keyboard and screen readers --- .../pipeline/PipelineDetails.tsx | 2 +- .../pipeline/SelectedTaskDrawerContent.tsx | 2 +- .../PipelineRecurringRunDetails.tsx | 2 +- .../pipelineRun/PipelineRunDetails.tsx | 2 +- .../PipelineRunDrawerRightContent.tsx | 7 +- .../PipelineRunDrawerRightTabs.tsx | 8 +- .../artifacts/ArtifactNodeDrawerContent.tsx | 2 +- .../__tests__/usePipelineTaskTopology.spec.ts | 1 + .../src/concepts/topology/NodeStatusIcon.tsx | 8 +- .../topology/PipelineDefaultTaskGroup.tsx | 217 +++++++++++++++--- .../concepts/topology/PipelineNodeButton.tsx | 56 +++++ .../PipelineVisualizationSurface.scss | 32 +++ .../topology/PipelineVisualizationSurface.tsx | 88 ++++++- .../__tests__/NodeStatusIcon.spec.tsx | 12 +- .../topology/__tests__/a11yUtils.spec.ts | 82 +++++++ frontend/src/concepts/topology/a11yUtils.ts | 29 +++ .../topology/customNodes/ArtifactTaskNode.tsx | 15 +- .../topology/customNodes/StandardTaskNode.tsx | 157 +++++++------ frontend/src/concepts/topology/factories.ts | 9 +- .../topology/useTopologyController.ts | 2 + frontend/src/concepts/topology/utils.ts | 53 +++++ .../cypress/pages/pipelines/topology.ts | 30 ++- .../topology/pipelinesTopology.cy.ts | 142 +++++++++++- .../pipeline_a11y_scenarios.py | 98 ++++++++ .../pipeline_a11y_scenarios.yaml | 213 +++++++++++++++++ 25 files changed, 1132 insertions(+), 137 deletions(-) create mode 100644 frontend/src/concepts/topology/PipelineNodeButton.tsx create mode 100644 frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts create mode 100644 frontend/src/concepts/topology/a11yUtils.ts create mode 100644 packages/cypress/resources/pipelines_samples/pipeline_a11y_scenarios.py create mode 100644 packages/cypress/resources/pipelines_samples/pipeline_a11y_scenarios.yaml diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx index aa545875ff8..70500bc8754 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/PipelineDetails.tsx @@ -89,7 +89,7 @@ const PipelineDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath }) = const panelContent = selectedNode ? ( setSelectedIds(undefined)} + onClose={() => setSelectedIds([])} /> ) : null; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/SelectedTaskDrawerContent.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/SelectedTaskDrawerContent.tsx index 60304a685c5..5d3128c4d7b 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/SelectedTaskDrawerContent.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipeline/SelectedTaskDrawerContent.tsx @@ -22,7 +22,7 @@ const SelectedTaskDrawerContent: React.FC = ({ t return ( <> - + <Title headingLevel="h2" size="xl" tabIndex={-1} data-testid="pipeline-drawer-task-title"> {task.name} {task.type === 'artifact' ? 'Artifact details' : ''} diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRecurringRun/PipelineRecurringRunDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRecurringRun/PipelineRecurringRunDetails.tsx index eec2c1f0644..7caa0e1714d 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRecurringRun/PipelineRecurringRunDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRecurringRun/PipelineRecurringRunDetails.tsx @@ -83,7 +83,7 @@ const PipelineRecurringRunDetails: PipelineCoreDetailsPageComponent = ({ const panelContent = selectedNode ? ( setSelectedIds(undefined)} + onClose={() => setSelectedIds([])} /> ) : null; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx index abb46436e28..428a36a1acf 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx @@ -135,7 +135,7 @@ const PipelineRunDetails: React.FC< setSelectedIds(undefined)} + onClose={() => setSelectedIds([])} executions={drawerExecutions} run={run} /> diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx index d65398f1f74..88b460d697b 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx @@ -48,7 +48,12 @@ const PipelineRunDrawerRightContent: React.FC - + <Title + headingLevel="h2" + size="xl" + tabIndex={-1} + data-testid="pipeline-drawer-task-title" + > {task.name} {task.status?.podName && ( diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx index 8bffe4b758b..1848f8ab0c0 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx @@ -81,7 +81,12 @@ const PipelineRunDrawerRightTabs: React.FC = ({ return ( <> - + setSelection(String(tabKey))} + > {Object.entries(tabs).map(([tabName, { title, isDisabled }]) => ( = ({ eventKey={tabName} tabContentId={tabName} isDisabled={isDisabled} - onClick={() => setSelection(tabName)} /> ))} diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactNodeDrawerContent.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactNodeDrawerContent.tsx index 7763dfa55f5..6dc6c73c054 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactNodeDrawerContent.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactNodeDrawerContent.tsx @@ -41,7 +41,7 @@ export const ArtifactNodeDrawerContent: React.FC return task ? ( <> - + <Title headingLevel="h2" size="xl" tabIndex={-1} data-testid="pipeline-drawer-task-title"> {task.name} {task.status?.podName && {task.status.podName}} diff --git a/frontend/src/concepts/pipelines/topology/__tests__/usePipelineTaskTopology.spec.ts b/frontend/src/concepts/pipelines/topology/__tests__/usePipelineTaskTopology.spec.ts index 50fca619795..6843f2c3085 100644 --- a/frontend/src/concepts/pipelines/topology/__tests__/usePipelineTaskTopology.spec.ts +++ b/frontend/src/concepts/pipelines/topology/__tests__/usePipelineTaskTopology.spec.ts @@ -14,6 +14,7 @@ jest.mock('@patternfly/react-topology', () => ({ Skipped: 'Skipped', }, observer: (c: unknown) => c, + action: (fn: (...args: unknown[]) => unknown) => fn, GraphComponent: {}, ModelKind: { graph: 'graph', node: 'node', edge: 'edge' }, ComponentFactory: {}, diff --git a/frontend/src/concepts/topology/NodeStatusIcon.tsx b/frontend/src/concepts/topology/NodeStatusIcon.tsx index 464b111f94c..6b4fdd95d73 100644 --- a/frontend/src/concepts/topology/NodeStatusIcon.tsx +++ b/frontend/src/concepts/topology/NodeStatusIcon.tsx @@ -11,7 +11,7 @@ import { RunStatus } from '@patternfly/react-topology'; import { Icon, Tooltip } from '@patternfly/react-core'; import { runtimeStateLabels, RuntimeStateKF } from '#~/concepts/pipelines/kfTypes'; -const NodeStatusIcon: React.FC<{ runStatus: RunStatus | string }> = ({ runStatus }) => { +const NodeStatusIcon: React.FC<{ runStatus?: RunStatus | string }> = ({ runStatus }) => { let icon: React.ReactNode; let status: React.ComponentProps['status']; let label: string; @@ -54,9 +54,13 @@ const NodeStatusIcon: React.FC<{ runStatus: RunStatus | string }> = ({ runStatus label = ''; } + if (!icon || !label) { + return null; + } + return ( - + {icon} diff --git a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx index e7414627e46..8c267fd88df 100644 --- a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx +++ b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx @@ -14,12 +14,18 @@ import { PipelineNodeModel, TaskGroupPillLabel, LabelPosition, + SELECTION_EVENT, + useVisualizationController, } from '@patternfly/react-topology'; -import { Flex, FlexItem, Popover, Stack, StackItem } from '@patternfly/react-core'; +import { Button, Flex, FlexItem, Popover, Stack, StackItem } from '@patternfly/react-core'; import { BanIcon } from '@patternfly/react-icons'; +import { t_global_spacer_lg as globalSpacerLg } from '@patternfly/react-tokens'; import { PipelineNodeModelExpanded, StandardTaskNodeData } from '#~/concepts/topology/types'; import NodeStatusIcon from '#~/concepts/topology/NodeStatusIcon'; import { ExecutionStateKF } from '#~/concepts/pipelines/kfTypes'; +import { getRunStatusLabel } from '#~/concepts/topology/utils'; +import { isHiddenByCollapsedAncestor } from '#~/concepts/topology/a11yUtils'; +import { activateNodeButtonOnKeyDown } from '#~/concepts/topology/PipelineNodeButton'; import { NODE_HEIGHT, NODE_WIDTH } from './const'; const MAX_TIP_ITEMS = 6; @@ -34,47 +40,139 @@ type PipelinesDefaultGroupInnerProps = Omit = observer( ({ element, selected, onSelect }) => { + const controller = useVisualizationController(); const [hover, hoverRef] = useHover(); - const popoverRef = React.useRef(null); + const [popoverOpen, setPopoverOpen] = React.useState(false); + const [toggleFocused, setToggleFocused] = React.useState(false); + const groupActionClassName = `odh-pipeline-task-group-${React.useId().replace(/:/g, '')}`; + const popoverRef = React.useRef(null); + const toggleRef = React.useRef(null); + const focusToggleAfterCollapse = React.useRef(false); const detailsLevel = element.getGraph().getDetailsLevel(); + const isCollapsed = element.isCollapsed(); + const isHidden = isHiddenByCollapsedAncestor(element); const runStatus = element.getData()?.runStatus; const state = element.getData()?.pipelineTask.status?.state; - const getPopoverTasksList = (items: Node[]) => ( + React.useEffect(() => { + if (!isCollapsed || isHidden) { + setPopoverOpen(false); + } + }, [isCollapsed, isHidden]); + + React.useEffect(() => { + if (focusToggleAfterCollapse.current) { + toggleRef.current?.focus(); + focusToggleAfterCollapse.current = false; + } + }, [isCollapsed]); + + React.useLayoutEffect(() => { + // Hide PatternFly's SVG label before paint so it never becomes a transient Tab stop. + document + .querySelectorAll( + `.${groupActionClassName} .pf-topology-pipelines__pill-text`, + ) + .forEach((label) => { + label.setAttribute('tabindex', '-1'); + label.setAttribute('aria-hidden', 'true'); + }); + }, [groupActionClassName, hover, isCollapsed, toggleFocused]); + + const activateCollapseAction = React.useCallback( + (event: React.MouseEvent) => { + event.stopPropagation(); + // DefaultTaskGroup owns collapse state, layout, and viewport updates. It exposes no + // imperative collapse action, so route this button through its SVG action icon. Keep + // the group-toggle Cypress test when updating PatternFly's action icon markup. + const actionIcon = document.querySelector( + `.${groupActionClassName} .pf-topology__node__action-icon`, + ); + if (actionIcon) { + focusToggleAfterCollapse.current = true; + actionIcon.dispatchEvent(new MouseEvent('click', { bubbles: true })); + } + }, + [groupActionClassName], + ); + + const selectChild = React.useCallback( + (childId: string) => { + controller.fireEvent(SELECTION_EVENT, [childId]); + }, + [controller], + ); + + const getPopoverTasksList = (items: Node[], hidePopover: () => void) => ( - {items.slice(0, MAX_TIP_ITEMS).map((item: Node) => ( - - - - - - {item.getLabel()} - - - ))} + {items.slice(0, MAX_TIP_ITEMS).map((item: Node) => { + const childRunStatus = item.getData()?.runStatus; + const childStatus = getRunStatusLabel(childRunStatus); + const childLabel = item.getLabel(); + return ( + + + + ); + })} {items.length > MAX_TIP_ITEMS ? ( {`... ${items.length - MAX_TIP_ITEMS} others`} ) : null} ); - const status = React.useMemo(() => { - switch (state) { - case ExecutionStateKF.CACHED: - return RunStatus.Succeeded; - case ExecutionStateKF.RUNNING: - return RunStatus.InProgress; - default: - return runStatus; - } - }, [state, runStatus]); + let status = runStatus; + if (state === ExecutionStateKF.CACHED) { + status = RunStatus.Succeeded; + } else if (state === ExecutionStateKF.RUNNING) { + status = RunStatus.InProgress; + } - const iterationCount = element.getData()?.pipelineTask.iterationCount; + const childCount = element.getAllNodeChildren().length; + const pipelineTask = element.getData()?.pipelineTask; + const iterationCount = + pipelineTask && + 'iterationCount' in pipelineTask && + typeof pipelineTask.iterationCount === 'number' + ? pipelineTask.iterationCount + : undefined; const badgeText = iterationCount != null ? `x${iterationCount}` : undefined; + const groupLabel = element.getLabel(); + const groupStatusLabel = getRunStatusLabel(status); + const groupAriaLabel = groupStatusLabel + ? `${groupLabel} task group, ${childCount} ${ + childCount === 1 ? 'task' : 'tasks' + }, ${groupStatusLabel}` + : `${groupLabel} task group, ${childCount} ${childCount === 1 ? 'task' : 'tasks'}`; + const bounds = element.getBounds(); const groupNode = ( ( @@ -90,7 +188,8 @@ const DefaultTaskGroupInner: React.FunctionComponent : undefined} showLabelOnHover status={status} @@ -107,19 +206,63 @@ const DefaultTaskGroupInner: React.FunctionComponent - {element.isCollapsed() ? ( - - {groupNode} - - ) : ( - groupNode - )} + setPopoverOpen(true)} + onHide={() => setPopoverOpen(false)} + onHidden={() => setPopoverOpen(false)} + aria-label={groupAriaLabel} + headerContent={groupLabel} + bodyContent={(hidePopover) => + getPopoverTasksList(element.getAllNodeChildren(), hidePopover) + } + > + + {items.slice(0, MAX_TIP_ITEMS).map((item: Node) => { const childState = item.getData()?.pipelineTask.status?.state; const childRunStatus = item.getData()?.runStatus; @@ -172,7 +188,7 @@ const DefaultTaskGroupInner: React.FunctionComponent { hidePopover(); - selectChild(item.getId()); + selectNode(item.getId()); }} onKeyDown={activateNodeButtonOnKeyDown} aria-label={ @@ -245,7 +261,7 @@ const DefaultTaskGroupInner: React.FunctionComponent : undefined} showLabelOnHover diff --git a/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts b/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts index 4534d96a2d3..2cf09fcf275 100644 --- a/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts +++ b/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts @@ -17,20 +17,35 @@ describe('orderNodesForKeyboard', () => { { graph: { id: 'graph', type: 'graph' }, nodes: [ - { id: 'last', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 200, width: 130, height: 35 }, + { id: 'z-last', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 200, width: 130, height: 35 }, { - id: 'group', + id: 'a-group', type: DEFAULT_TASK_NODE_TYPE, - x: 0, + x: 200, y: 100, width: 130, height: 100, group: true, - children: ['child-last', 'child-first'], + style: { padding: [64, 15] }, + children: ['a-child-last', 'z-child-first'], + }, + { + id: 'a-child-last', + type: DEFAULT_TASK_NODE_TYPE, + x: 200, + y: 160, + width: 130, + height: 35, + }, + { id: 'z-first', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 100, width: 130, height: 35 }, + { + id: 'z-child-first', + type: DEFAULT_TASK_NODE_TYPE, + x: 200, + y: 100, + width: 130, + height: 35, }, - { id: 'child-last', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 60, width: 130, height: 35 }, - { id: 'first', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 0, width: 130, height: 35 }, - { id: 'child-first', type: DEFAULT_TASK_NODE_TYPE, x: 0, y: 0, width: 130, height: 35 }, ], }, false, @@ -39,13 +54,13 @@ describe('orderNodesForKeyboard', () => { const graph = controller.getGraph(); orderNodesForKeyboard(graph); - expect(graph.getNodes().map((node) => node.getId())).toEqual(['first', 'group', 'last']); + expect(graph.getNodes().map((node) => node.getId())).toEqual(['z-first', 'a-group', 'z-last']); expect( controller - .getNodeById('group') + .getNodeById('a-group') ?.getNodes() .map((node) => node.getId()), - ).toEqual(['child-first', 'child-last']); + ).toEqual(['z-child-first', 'a-child-last']); }); }); diff --git a/frontend/src/concepts/topology/a11yUtils.ts b/frontend/src/concepts/topology/a11yUtils.ts index 5f4ed0e6ccd..4199ca2cb95 100644 --- a/frontend/src/concepts/topology/a11yUtils.ts +++ b/frontend/src/concepts/topology/a11yUtils.ts @@ -62,13 +62,33 @@ export const isHiddenByCollapsedAncestor = (element: Node): boolean => { return false; }; +const getReadingOrderPosition = (node: Node): { x: number; y: number } => { + const bounds = node.getBounds(); + if (!node.isGroup() || node.isCollapsed()) { + return bounds; + } + + const visibleChildren = node.getNodes().filter((child) => child.isVisible()); + if (!visibleChildren.length) { + return bounds; + } + + return visibleChildren + .map(getReadingOrderPosition) + .reduce((first, child) => + child.y < first.y || (child.y === first.y && child.x < first.x) ? child : first, + ); +}; + /** Keep the graph's DOM order aligned with the laid-out pipeline, including nested groups. */ export const orderNodesForKeyboard = action((parent: Graph | Node): void => { const nodes = parent.getNodes(); const orderedNodes = nodes.toSorted((a, b) => { - const aBounds = a.getBounds(); - const bBounds = b.getBounds(); - return aBounds.y - bBounds.y || aBounds.x - bBounds.x || a.getId().localeCompare(b.getId()); + const aPosition = getReadingOrderPosition(a); + const bPosition = getReadingOrderPosition(b); + return ( + aPosition.y - bPosition.y || aPosition.x - bPosition.x || a.getId().localeCompare(b.getId()) + ); }); if (nodes.some((node, index) => node !== orderedNodes[index])) { diff --git a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx index 978234dfa21..ff718770ed9 100644 --- a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx +++ b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx @@ -55,10 +55,6 @@ const IconTaskNode: React.FC = observer(({ element, selected, AnchorEnd.target, ); - const statusLabel = getRunStatusLabel(data?.runStatus); - const taskName = element.getLabel(); - const ariaLabel = statusLabel ? `${taskName}, ${statusLabel}` : taskName; - return ( = observer(({ element, selected, )} - ); }); @@ -173,11 +168,11 @@ const ArtifactTaskNodeInner: React.FC = observer( ) : null} - ) : ( )} + ); }, diff --git a/packages/cypress/cypress/pages/pipelines/topology.ts b/packages/cypress/cypress/pages/pipelines/topology.ts index da42e4c8d5b..dcb107d9c0d 100644 --- a/packages/cypress/cypress/pages/pipelines/topology.ts +++ b/packages/cypress/cypress/pages/pipelines/topology.ts @@ -79,6 +79,10 @@ class PipelinesTopology { return cy.findByTestId(`pipeline-group-task-${name}`); } + findGroupDetails(name: string) { + return cy.findByTestId(`pipeline-group-details-${name}`); + } + findGraphButtons() { return cy.get('.odh-pipeline-node-button'); } diff --git a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts index 697244f194f..fbcac054a0e 100644 --- a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts +++ b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts @@ -399,6 +399,23 @@ describe('Pipeline topology', () => { pipelineDetails.findGroupButton('for-loop-2').should('be.focused'); }); + it('opens group details from the collapsed-group popover', () => { + initIntercepts(mockGroupedVersion); + pipelineDetails.visit( + projectId, + mockPipeline.pipeline_id, + mockGroupedVersion.pipeline_version_id, + ); + + pipelineDetails.findGroupButton('for-loop-2').focus(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findGroupDetails('for-loop-2').focus(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findDrawerTaskTitle().should('be.focused'); + pipelineDetails.findDrawerCloseButton().click(); + pipelineDetails.findGroupButton('for-loop-2').should('be.focused'); + }); + it('uses PatternFly collapse behavior from the keyboard accessible control', () => { initIntercepts(mockGroupedVersion); pipelineDetails.visit( @@ -486,6 +503,29 @@ describe('Pipeline topology', () => { pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').should('be.focused'); }); + it('keeps focus on an artifact when hovering changes its visual node', () => { + initIntercepts(); + pipelineRunDetails.visit(projectId, mockRun.run_id); + + for (let i = 0; i < 8; i++) { + pipelineRunDetails.findControlBar().findByRole('button', { name: 'Zoom In' }).click(); + } + + pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').focus(); + pipelineRunDetails + .findTaskButton('iris_dataset (Type: Dataset)') + .closest('[data-kind="node"]') + .find('.pf-topology-pipelines__pill-background') + .should('exist'); + pipelineRunDetails + .findTaskButton('iris_dataset (Type: Dataset)') + .closest('[data-kind="node"]') + .find('g.pf-topology__pipelines__task-node') + .first() + .trigger('mouseenter'); + pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').should('be.focused'); + }); + it('preserves graph keyboard order after selecting a step', () => { initIntercepts(); pipelineRunDetails.visit(projectId, mockRun.run_id); From 4c455e77271364f486e4bf79aa118ca85b37576f Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Thu, 15 Oct 2026 17:07:39 -0400 Subject: [PATCH 09/17] test(RHOAIENG-66823): use stable artifact hover selectors --- .../topology/customNodes/ArtifactTaskNode.tsx | 7 ++++++- .../cypress/cypress/pages/pipelines/topology.ts | 8 ++++++++ .../pipelines/topology/pipelinesTopology.cy.ts | 14 +++----------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx index ff718770ed9..37181ba3249 100644 --- a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx +++ b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx @@ -57,6 +57,7 @@ const IconTaskNode: React.FC = observer(({ element, selected, return ( = observer( const ariaLabel = statusLabel ? `${taskName}, ${statusLabel}` : taskName; return ( - + {isHover || detailsLevel !== ScaleDetailsLevel.high ? ( { diff --git a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts index fbcac054a0e..978847d30b6 100644 --- a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts +++ b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts @@ -512,17 +512,9 @@ describe('Pipeline topology', () => { } pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').focus(); - pipelineRunDetails - .findTaskButton('iris_dataset (Type: Dataset)') - .closest('[data-kind="node"]') - .find('.pf-topology-pipelines__pill-background') - .should('exist'); - pipelineRunDetails - .findTaskButton('iris_dataset (Type: Dataset)') - .closest('[data-kind="node"]') - .find('g.pf-topology__pipelines__task-node') - .first() - .trigger('mouseenter'); + pipelineRunDetails.findArtifactIcon('iris_dataset (Type: Dataset)').should('exist'); + pipelineRunDetails.findArtifactVisual('iris_dataset (Type: Dataset)').trigger('mouseenter'); + pipelineRunDetails.findArtifactIcon('iris_dataset (Type: Dataset)').should('not.exist'); pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').should('be.focused'); }); From 1529955c92becba25c748076cd197860467063d4 Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Fri, 16 Oct 2026 15:21:32 -0400 Subject: [PATCH 10/17] fix: preserve focus when opening pipeline group details --- frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx index 4eed9246ffd..b5a7a1caf93 100644 --- a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx +++ b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx @@ -166,11 +166,11 @@ const DefaultTaskGroupInner: React.FunctionComponent { hidePopover(); - popoverRef.current?.focus(); selectNode(element.getId()); }} onKeyDown={activateNodeButtonOnKeyDown} aria-label={`View details for ${element.getLabel()} task group`} + data-pipeline-node-id={element.getId()} data-testid={`pipeline-group-details-${element.getLabel()}`} > View group details From 029b5bef164601723903db7d4356ee45943d1e35 Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Tue, 20 Oct 2026 10:24:53 -0400 Subject: [PATCH 11/17] fix(pipelines): address topology accessibility review findings --- .../src/concepts/topology/NodeStatusIcon.tsx | 2 +- .../topology/PipelineDefaultTaskGroup.tsx | 39 ++++++----- .../concepts/topology/PipelineNodeButton.tsx | 17 +++-- .../topology/PipelineVisualizationSurface.tsx | 15 +++-- .../__tests__/PipelineNodeButton.spec.tsx | 21 ++++++ .../topology/__tests__/a11yUtils.spec.ts | 18 +++++- .../concepts/topology/__tests__/utils.spec.ts | 46 ++++++++++++- frontend/src/concepts/topology/a11yUtils.ts | 4 +- .../topology/customNodes/ArtifactTaskNode.tsx | 14 ++-- .../cypress/pages/pipelines/topology.ts | 5 ++ .../topology/pipelinesTopology.cy.ts | 64 ++++++++++++++++++- 11 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 frontend/src/concepts/topology/__tests__/PipelineNodeButton.spec.tsx diff --git a/frontend/src/concepts/topology/NodeStatusIcon.tsx b/frontend/src/concepts/topology/NodeStatusIcon.tsx index 6350dec794c..afe01171f34 100644 --- a/frontend/src/concepts/topology/NodeStatusIcon.tsx +++ b/frontend/src/concepts/topology/NodeStatusIcon.tsx @@ -73,7 +73,7 @@ const NodeStatusIcon: React.FC<{ return ( - + {icon} diff --git a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx index b5a7a1caf93..bd313d6e1fe 100644 --- a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx +++ b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx @@ -27,13 +27,13 @@ import { getPipelineTaskStatusLabel, getRunStatusLabel } from '#~/concepts/topol import { hideSvgTaskLabelsFromKeyboard, isHiddenByCollapsedAncestor, - panGroupControlIntoView, - panNodeIntoView, + panFocusedControlIntoView, } from '#~/concepts/topology/a11yUtils'; import { activateNodeButtonOnKeyDown } from '#~/concepts/topology/PipelineNodeButton'; import { NODE_HEIGHT, NODE_WIDTH } from './const'; const MAX_TIP_ITEMS = 6; +const EXPANDED_TOGGLE_OFFSET_FROM_CENTER = NODE_WIDTH / 2 - 42.5; type PipelinesDefaultGroupProps = { element: GraphElement; @@ -44,10 +44,12 @@ type PipelinesDefaultGroupInnerProps = Omit & { + groupLabel: string; onActionChange: (handler: ((event: React.MouseEvent) => void) | undefined) => void; }; const AccessibleTaskGroupPillLabel: React.FC = ({ + groupLabel, onActionChange, ...props }) => { @@ -73,7 +75,7 @@ const AccessibleTaskGroupPillLabel: React.FC }, []); return ( - + ); @@ -241,20 +243,27 @@ const DefaultTaskGroupInner: React.FunctionComponent) => ( + : undefined} + /> + ), + [badgeText, groupLabel, setExpandedAction, status], + ); const groupNode = ( ( - : undefined} - /> - )} + GroupLabelComponent={GroupLabelComponent} selected={selected} onSelect={onSelect} hideDetailsAtMedium @@ -305,7 +314,7 @@ const DefaultTaskGroupInner: React.FunctionComponent panNodeIntoView(element)} + onFocus={(event) => panFocusedControlIntoView(element, event.currentTarget)} onClick={(event) => event.stopPropagation()} onKeyDown={activateNodeButtonOnKeyDown} data-pipeline-node-id={element.getId()} @@ -316,7 +325,7 @@ const DefaultTaskGroupInner: React.FunctionComponent { setToggleFocused(true); if (toggleRef.current) { - panGroupControlIntoView(element, toggleRef.current); + panFocusedControlIntoView(element, toggleRef.current); } }} onBlur={() => setToggleFocused(false)} diff --git a/frontend/src/concepts/topology/PipelineNodeButton.tsx b/frontend/src/concepts/topology/PipelineNodeButton.tsx index a7b9bba13e2..725e3b4e7fa 100644 --- a/frontend/src/concepts/topology/PipelineNodeButton.tsx +++ b/frontend/src/concepts/topology/PipelineNodeButton.tsx @@ -1,11 +1,12 @@ import React from 'react'; import { Node, observer, WithSelectionProps } from '@patternfly/react-topology'; -import { isHiddenByCollapsedAncestor, panNodeIntoView } from './a11yUtils'; +import { isHiddenByCollapsedAncestor, panFocusedControlIntoView } from './a11yUtils'; type PipelineNodeButtonProps = { element: Node; ariaLabel: string; onSelect: WithSelectionProps['onSelect']; + onFocusChange?: (focused: boolean) => void; }; // Chrome does not synthesize button clicks from keyboard input inside an SVG foreignObject. @@ -14,13 +15,17 @@ export const activateNodeButtonOnKeyDown = ( ): void => { if (event.key === 'Enter' || event.key === ' ') { event.preventDefault(); - event.currentTarget.click(); + if (!event.repeat) { + event.currentTarget.click(); + } } }; /** A native keyboard target over a selectable SVG node; pointer clicks stay on the SVG. */ +// TODO: Reassess custom node and group tab stops when +// https://github.com/patternfly/react-topology/pull/322 reaches our installed version. const PipelineNodeButton: React.FC = observer( - ({ element, ariaLabel, onSelect }) => { + ({ element, ariaLabel, onSelect, onFocusChange }) => { const bounds = element.getBounds(); if (isHiddenByCollapsedAncestor(element)) { @@ -40,7 +45,11 @@ const PipelineNodeButton: React.FC = observer( type="button" className="odh-pipeline-node-button pf-v6-u-display-block pf-v6-u-w-100 pf-v6-u-h-100" aria-label={ariaLabel} - onFocus={() => panNodeIntoView(element)} + onFocus={(event) => { + panFocusedControlIntoView(element, event.currentTarget); + onFocusChange?.(true); + }} + onBlur={() => onFocusChange?.(false)} onClick={(event) => { event.stopPropagation(); onSelect?.(event); diff --git a/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx b/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx index ac9ae9414da..f540b673b7f 100644 --- a/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx +++ b/frontend/src/concepts/topology/PipelineVisualizationSurface.tsx @@ -95,8 +95,6 @@ const PipelineVisualizationSurface: React.FC () => (selectedIds?.[0] ? controller.getNodeById(selectedIds[0]) || null : null), [selectedIds, controller], ); - const selectedNodeIsHidden = selectedNode ? isHiddenByCollapsedAncestor(selectedNode) : false; - const selections = React.useMemo(() => { if (selectedIds?.[0]) { const element = controller.getElementById(selectedIds[0]); @@ -138,7 +136,16 @@ const PipelineVisualizationSurface: React.FC React.useEffect(() => { // A collapsed group hides its child's graph button; choosing that child also closes its // popover, so move focus to the drawer even though the original button is now gone. - if (!selectedId || (!focusReturnTarget.current && !selectedNodeIsHidden)) { + // Check visibility only when selection changes: collapsing the group later must leave + // focus on the group toggle instead of sending it back to an already open drawer. + if (!selectedId) { + return undefined; + } + const selectedNodeAtSelection = controller.getNodeById(selectedId); + const hiddenAtSelection = selectedNodeAtSelection + ? isHiddenByCollapsedAncestor(selectedNodeAtSelection) + : false; + if (!focusReturnTarget.current && !hiddenAtSelection) { return undefined; } const focusTimeout = setTimeout(() => { @@ -151,7 +158,7 @@ const PipelineVisualizationSurface: React.FC focusTarget?.focus(); }, 550); return () => clearTimeout(focusTimeout); - }, [selectedId, selectedNodeIsHidden]); + }, [controller, selectedId]); React.useEffect(() => { const currentModel = controller.toModel(); diff --git a/frontend/src/concepts/topology/__tests__/PipelineNodeButton.spec.tsx b/frontend/src/concepts/topology/__tests__/PipelineNodeButton.spec.tsx new file mode 100644 index 00000000000..5842aa87347 --- /dev/null +++ b/frontend/src/concepts/topology/__tests__/PipelineNodeButton.spec.tsx @@ -0,0 +1,21 @@ +import React from 'react'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { activateNodeButtonOnKeyDown } from '#~/concepts/topology/PipelineNodeButton'; + +describe('activateNodeButtonOnKeyDown', () => { + it.each(['Enter', ' '])('activates once when %s is held', (key) => { + const onClick = jest.fn(); + render( + , + ); + const button = screen.getByRole('button', { name: 'Toggle group' }); + + fireEvent.keyDown(button, { key }); + fireEvent.keyDown(button, { key, repeat: true }); + fireEvent.keyDown(button, { key, repeat: true }); + + expect(onClick).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts b/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts index 2cf09fcf275..740ace9cecc 100644 --- a/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts +++ b/frontend/src/concepts/topology/__tests__/a11yUtils.spec.ts @@ -6,7 +6,7 @@ import { import { isHiddenByCollapsedAncestor, orderNodesForKeyboard, - panGroupControlIntoView, + panFocusedControlIntoView, } from '#~/concepts/topology/a11yUtils'; describe('orderNodesForKeyboard', () => { @@ -100,7 +100,7 @@ describe('isHiddenByCollapsedAncestor', () => { }); }); -describe('panGroupControlIntoView', () => { +describe('panFocusedControlIntoView', () => { it('pans an offscreen group control into the topology viewport', () => { const controller = new Visualization(); controller.registerElementFactory(pipelineElementFactory); @@ -136,10 +136,22 @@ describe('panGroupControlIntoView', () => { } as DOMRect); const positionBefore = controller.getGraph().getPosition().clone(); - panGroupControlIntoView(group, button); + panFocusedControlIntoView(group, button); expect(controller.getGraph().getPosition().x).toBe(positionBefore.x + 40); expect(controller.getGraph().getPosition().y).toBe(positionBefore.y + 80); + + // A partly clipped control should also move fully inside the padded viewport. + jest.spyOn(button, 'getBoundingClientRect').mockReturnValue({ + left: 110, + right: 240, + top: 360, + bottom: 390, + } as DOMRect); + const positionBeforePartialPan = controller.getGraph().getPosition().clone(); + panFocusedControlIntoView(group, button); + expect(controller.getGraph().getPosition().x).toBe(positionBeforePartialPan.x + 10); + expect(controller.getGraph().getPosition().y).toBe(positionBeforePartialPan.y - 10); viewport.remove(); }); }); diff --git a/frontend/src/concepts/topology/__tests__/utils.spec.ts b/frontend/src/concepts/topology/__tests__/utils.spec.ts index 0e04f1fd782..aec593a6257 100644 --- a/frontend/src/concepts/topology/__tests__/utils.spec.ts +++ b/frontend/src/concepts/topology/__tests__/utils.spec.ts @@ -1,6 +1,46 @@ import { RunStatus } from '@patternfly/react-topology'; import { ExecutionStateKF } from '#~/concepts/pipelines/kfTypes'; -import { getPipelineTaskStatusLabel } from '#~/concepts/topology/utils'; +import { + getExecutionStateLabel, + getPipelineTaskStatusLabel, + getRunStatusLabel, +} from '#~/concepts/topology/utils'; + +describe('getRunStatusLabel', () => { + it.each([ + [RunStatus.Succeeded, 'Complete'], + [RunStatus.Failed, 'Failed'], + [RunStatus.FailedToStart, 'Failed'], + [RunStatus.Cancelled, 'Canceled'], + [RunStatus.Running, 'Running'], + [RunStatus.InProgress, 'Running'], + [RunStatus.Pending, 'Pending'], + [RunStatus.Idle, 'Pending'], + [RunStatus.Skipped, 'Skipped'], + ])('announces %s as %s', (status, label) => { + expect(getRunStatusLabel(status)).toBe(label); + }); + + it('omits an unavailable status', () => { + expect(getRunStatusLabel(undefined)).toBe(''); + }); +}); + +describe('getExecutionStateLabel', () => { + it.each([ + [ExecutionStateKF.CACHED, 'Cached'], + [ExecutionStateKF.CANCELING, 'Canceling'], + [ExecutionStateKF.CANCELED, 'Canceled'], + [ExecutionStateKF.SKIPPED, 'Skipped'], + [ExecutionStateKF.PENDING, 'Pending'], + [ExecutionStateKF.RUNNING, 'Running'], + [ExecutionStateKF.COMPLETE, 'Complete'], + [ExecutionStateKF.FAILED, 'Failed'], + [ExecutionStateKF.NEW, undefined], + ])('announces %s as %s', (state, label) => { + expect(getExecutionStateLabel(state)).toBe(label); + }); +}); describe('getPipelineTaskStatusLabel', () => { it('announces cached tasks as cached even when their run status is skipped', () => { @@ -10,4 +50,8 @@ describe('getPipelineTaskStatusLabel', () => { it('uses run status when no execution state is available', () => { expect(getPipelineTaskStatusLabel(undefined, RunStatus.Failed)).toBe('Failed'); }); + + it('uses run status when an execution state has no spoken label', () => { + expect(getPipelineTaskStatusLabel(ExecutionStateKF.NEW, RunStatus.Pending)).toBe('Pending'); + }); }); diff --git a/frontend/src/concepts/topology/a11yUtils.ts b/frontend/src/concepts/topology/a11yUtils.ts index 4199ca2cb95..c62f21f891e 100644 --- a/frontend/src/concepts/topology/a11yUtils.ts +++ b/frontend/src/concepts/topology/a11yUtils.ts @@ -15,8 +15,8 @@ export const panNodeIntoView = (element: Node): void => { element.getGraph().panIntoView(element, { offset: 20, minimumVisible: 100 }); }; -/** Pan a focused group control into the graph viewport when the group itself is larger. */ -export const panGroupControlIntoView = (element: Node, button: HTMLButtonElement): void => { +/** Keep the whole focused control visible, including when its node is partly in view. */ +export const panFocusedControlIntoView = (element: Node, button: HTMLButtonElement): void => { const viewport = button.closest('[data-test-id="topology"]'); if (!viewport) { panNodeIntoView(element); diff --git a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx index 37181ba3249..44cea3fa820 100644 --- a/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx +++ b/frontend/src/concepts/topology/customNodes/ArtifactTaskNode.tsx @@ -109,6 +109,7 @@ const ArtifactTaskNodeInner: React.FC = observer( ({ element, selected, onSelect, ...rest }) => { const bounds = element.getBounds(); const [isHover, hoverRef] = useHover(); + const [isFocused, setIsFocused] = React.useState(false); const nodeRef = React.useRef(null); const combinedRef = useCombineRefs(hoverRef, nodeRef); React.useLayoutEffect(() => hideSvgTaskLabelsFromKeyboard(nodeRef.current)); @@ -140,7 +141,7 @@ const ArtifactTaskNodeInner: React.FC = observer( data-testid={`pipeline-artifact-visual-${element.getLabel()}`} ref={combinedRef} > - {isHover || detailsLevel !== ScaleDetailsLevel.high ? ( + {isHover || isFocused || detailsLevel !== ScaleDetailsLevel.high ? ( = observer( onSelect={onSelect} hiddenDetailsShownStatuses={[]} status={data?.runStatus} - scaleNode={isHover} + scaleNode={isHover || isFocused} {...rest} > {whenDecorator} - {!isHover ? ( + {!isHover && !isFocused ? ( = observer( ) : ( )} - + ); }, diff --git a/packages/cypress/cypress/pages/pipelines/topology.ts b/packages/cypress/cypress/pages/pipelines/topology.ts index def08d87d22..807959e87bd 100644 --- a/packages/cypress/cypress/pages/pipelines/topology.ts +++ b/packages/cypress/cypress/pages/pipelines/topology.ts @@ -71,6 +71,11 @@ class PipelinesTopology { return cy.findByTestId(`pipeline-group-toggle-${name}`); } + findGroupActionIcon(name: string) { + // PatternFly owns this SVG action icon; scope its class selector to the named group. + return cy.findByTestId(`pipeline-group-label-${name}`).find('.pf-topology__node__action-icon'); + } + findGroupButton(name: string) { return cy.findByTestId(`pipeline-group-button-${name}`); } diff --git a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts index 978847d30b6..d5bd64b3fc4 100644 --- a/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts +++ b/packages/cypress/cypress/tests/mocked/pipelines/topology/pipelinesTopology.cy.ts @@ -19,7 +19,10 @@ import { RecurringRunStatus, RuntimeStateKF, } from '@odh-dashboard/internal/concepts/pipelines/kfTypes'; -import { mockParallelForPipelineSpec } from '@odh-dashboard/internal/concepts/pipelines/topology/__tests__/mockParallelForPipelineSpec'; +import { + mockParallelForPipelineSpec, + mockParallelForWithArtifactsPipelineSpec, +} from '@odh-dashboard/internal/concepts/pipelines/topology/__tests__/mockParallelForPipelineSpec'; import { DataScienceStackComponent } from '@odh-dashboard/plugin-core/areas'; import { SecretModel, PodModel } from '@odh-dashboard/k8s-core/api/models'; import { @@ -59,6 +62,11 @@ const mockGroupedVersion = buildMockPipelineVersion({ display_name: 'grouped-pipeline-version', }); mockGroupedVersion.pipeline_spec = mockParallelForPipelineSpec; +const mockWideGroupedVersion = buildMockPipelineVersion({ + pipeline_id: mockPipeline.pipeline_id, + display_name: 'wide-grouped-pipeline-version', +}); +mockWideGroupedVersion.pipeline_spec = mockParallelForWithArtifactsPipelineSpec; const mockRun = buildMockRunKF({ display_name: 'test-pipeline-run', run_id: 'test-pipeline-run-id', @@ -440,6 +448,55 @@ describe('Pipeline topology', () => { pipelineDetails.findTaskButton('simple-task').should('not.exist'); }); + it('keeps focus on the group toggle when hiding a selected child', () => { + initIntercepts(mockGroupedVersion); + pipelineDetails.visit( + projectId, + mockPipeline.pipeline_id, + mockGroupedVersion.pipeline_version_id, + ); + + pipelineDetails.findGroupToggle('for-loop-2').focus(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findTaskButton('simple-task').focus(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findDrawerTaskTitle().should('be.focused'); + + pipelineDetails.findGroupToggle('for-loop-2').focus(); + cy.clock(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findTaskButton('simple-task').should('not.exist'); + pipelineDetails.findGroupToggle('for-loop-2').should('be.focused'); + cy.tick(600); + pipelineDetails.findGroupToggle('for-loop-2').should('be.focused'); + }); + + it('keeps the expanded group toggle aligned with its SVG action', () => { + initIntercepts(mockWideGroupedVersion); + pipelineDetails.visit( + projectId, + mockPipeline.pipeline_id, + mockWideGroupedVersion.pipeline_version_id, + ); + + pipelineDetails.findGroupToggle('for-loop-2').focus(); + cy.press(Cypress.Keyboard.Keys.ENTER); + pipelineDetails.findTaskButton('create-features').should('exist'); + pipelineDetails.findGroupActionIcon('for-loop-2').should('have.length', 1); + pipelineDetails.findGroupToggle('for-loop-2').then(($toggle) => { + pipelineDetails.findGroupActionIcon('for-loop-2').then(($action) => { + const toggle = $toggle[0].getBoundingClientRect(); + const action = $action[0].getBoundingClientRect(); + expect( + Math.min(toggle.right, action.right) - Math.max(toggle.left, action.left), + ).to.be.greaterThan(0); + expect( + Math.min(toggle.bottom, action.bottom) - Math.max(toggle.top, action.top), + ).to.be.greaterThan(0); + }); + }); + }); + it('keeps the expanded group label out of keyboard navigation', () => { initIntercepts(mockGroupedVersion); pipelineDetails.visit( @@ -503,7 +560,7 @@ describe('Pipeline topology', () => { pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').should('be.focused'); }); - it('keeps focus on an artifact when hovering changes its visual node', () => { + it('shows an artifact label on focus and keeps focus through hover', () => { initIntercepts(); pipelineRunDetails.visit(projectId, mockRun.run_id); @@ -511,8 +568,9 @@ describe('Pipeline topology', () => { pipelineRunDetails.findControlBar().findByRole('button', { name: 'Zoom In' }).click(); } - pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').focus(); pipelineRunDetails.findArtifactIcon('iris_dataset (Type: Dataset)').should('exist'); + pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').focus(); + pipelineRunDetails.findArtifactIcon('iris_dataset (Type: Dataset)').should('not.exist'); pipelineRunDetails.findArtifactVisual('iris_dataset (Type: Dataset)').trigger('mouseenter'); pipelineRunDetails.findArtifactIcon('iris_dataset (Type: Dataset)').should('not.exist'); pipelineRunDetails.findTaskButton('iris_dataset (Type: Dataset)').should('be.focused'); From 48975be54edee21e8b72a782c6602658f808dd90 Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Tue, 20 Oct 2026 10:47:58 -0400 Subject: [PATCH 12/17] fix(pipelines): align group controls and preserve keyboard focus --- .../topology/PipelineDefaultTaskGroup.tsx | 115 ++++++++++++++++-- .../topology/PipelineVisualizationSurface.tsx | 10 ++ .../cypress/pages/pipelines/topology.ts | 4 + .../topology/pipelinesTopology.cy.ts | 92 +++++++++++--- 4 files changed, 191 insertions(+), 30 deletions(-) diff --git a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx index bd313d6e1fe..3d347bafe6b 100644 --- a/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx +++ b/frontend/src/concepts/topology/PipelineDefaultTaskGroup.tsx @@ -35,6 +35,10 @@ import { NODE_HEIGHT, NODE_WIDTH } from './const'; const MAX_TIP_ITEMS = 6; const EXPANDED_TOGGLE_OFFSET_FROM_CENTER = NODE_WIDTH / 2 - 42.5; +type ControlBounds = { x: number; y: number; width: number; height: number }; + +type ExpandedControlBounds = { details: ControlBounds; toggle: ControlBounds }; + type PipelinesDefaultGroupProps = { element: GraphElement; } & WithSelectionProps; @@ -46,11 +50,13 @@ type PipelinesDefaultGroupInnerProps = Omit & { groupLabel: string; onActionChange: (handler: ((event: React.MouseEvent) => void) | undefined) => void; + onLabelLayoutChange: (label: SVGGElement) => void; }; const AccessibleTaskGroupPillLabel: React.FC = ({ groupLabel, onActionChange, + onLabelLayoutChange, ...props }) => { const labelRef = React.useRef(null); @@ -68,11 +74,20 @@ const AccessibleTaskGroupPillLabel: React.FC return undefined; } - hideSvgTaskLabelsFromKeyboard(label); - const labelObserver = new MutationObserver(() => hideSvgTaskLabelsFromKeyboard(label)); - labelObserver.observe(label, { childList: true, subtree: true }); + const updateLabel = () => { + hideSvgTaskLabelsFromKeyboard(label); + onLabelLayoutChange(label); + }; + updateLabel(); + const labelObserver = new MutationObserver(updateLabel); + labelObserver.observe(label, { + attributes: true, + attributeFilter: ['transform', 'x', 'y', 'width', 'height'], + childList: true, + subtree: true, + }); return () => labelObserver.disconnect(); - }, []); + }, [onLabelLayoutChange]); return ( @@ -89,6 +104,8 @@ const DefaultTaskGroupInner: React.FunctionComponent(null); const popoverRef = React.useRef(null); const toggleRef = React.useRef(null); const expandedActionRef = React.useRef<((event: React.MouseEvent) => void) | undefined>(); @@ -98,6 +115,48 @@ const DefaultTaskGroupInner: React.FunctionComponent { + const group = groupRef.current; + const actionIcon = label.querySelector('.pf-topology__node__action-icon'); + const matrix = group?.getScreenCTM(); + if (!actionIcon || !matrix) { + return; + } + + const inverse = matrix.inverse(); + const labelRect = label.getBoundingClientRect(); + const actionRect = actionIcon.getBoundingClientRect(); + if (!labelRect.width || !actionRect.width) { + return; + } + const toLocal = (rect: DOMRect): ControlBounds => { + const start = new DOMPoint(rect.left, rect.top).matrixTransform(inverse); + const end = new DOMPoint(rect.right, rect.bottom).matrixTransform(inverse); + return { x: start.x, y: start.y, width: end.x - start.x, height: end.y - start.y }; + }; + const details = toLocal( + new DOMRect( + labelRect.left, + labelRect.top, + actionRect.left - labelRect.left, + labelRect.height, + ), + ); + const toggle = toLocal(actionRect); + setExpandedControlBounds((previous) => { + const next = { details, toggle }; + const sameBounds = (a: ControlBounds, b: ControlBounds) => + Math.abs(a.x - b.x) < 0.1 && + Math.abs(a.y - b.y) < 0.1 && + Math.abs(a.width - b.width) < 0.1 && + Math.abs(a.height - b.height) < 0.1; + return previous && + sameBounds(previous.details, details) && + sameBounds(previous.toggle, toggle) + ? previous + : next; + }); + }, []); const focusToggleAfterCollapse = React.useRef(false); const detailsLevel = element.getGraph().getDetailsLevel(); const isCollapsed = element.isCollapsed(); @@ -243,19 +302,32 @@ const DefaultTaskGroupInner: React.FunctionComponent) => ( : undefined} /> ), - [badgeText, groupLabel, setExpandedAction, status], + [badgeText, groupLabel, setExpandedAction, status, updateExpandedControlBounds], ); const groupNode = ( @@ -323,12 +395,33 @@ const DefaultTaskGroupInner: React.FunctionComponent ) : null} + {!isCollapsed && !isHidden ? ( + +