From f0372c4219f67fc22fe03e1db0d4625958e68b3a Mon Sep 17 00:00:00 2001 From: Blai Peidro Date: Wed, 26 Aug 2026 22:38:03 +0200 Subject: [PATCH 1/3] fix: make the workflow job selector show and switch the right job The selector in the job tab bar always read "Workflow Job 1/X", whatever task you were on, and picking another task did nothing. Closes #742. Three separate defects, only the first of which was in the selector's own counter: - The position was the literal 1, and the list it counted excluded the job on screen, so a four task workflow read 1/3 on every one of its tasks. Count every node that ran a job, the current one included, and report where in that list the job on screen sits. - Picking a task did nothing because the selector is registered in Job.js as a tab with no link, so that it sits in the tab bar. The click bubbles to the PatternFly tab, and RoutedTabs called navigate(undefined) for it, landing back on the current url and undoing the navigation the selector had just performed. RoutedTabs now leaves link-less tabs alone. - RoutedTabs passed href={!tab.hasstyle && `#${tab.link}`}, which is the boolean false for exactly that tab. React rejects false as an href. Two more found while testing the above: - With a status filter applied, the toggle rendered a Label with a close button inside the toggle's own button. Nested buttons are invalid, and that close button was unclickable for the same bubbling reason. The filter is cleared by picking the same status again. - The menu seeded useState from the first render, so after moving between jobs it still offered the list belonging to the page you came from. The list is derived now. The old component test mocked out the component under test and asserted against the mock, which is why none of this was caught. It is replaced by twelve tests against the real component, plus two on RoutedTabs covering tabs that host a control. --- .../src/components/RoutedTabs/RoutedTabs.js | 22 +- .../components/RoutedTabs/RoutedTabs.test.js | 50 ++++ .../WorkflowOutputNavigation.js | 136 +++++----- .../WorkflowOutputNavigation.test.js | 255 +++++++++++++----- 4 files changed, 324 insertions(+), 139 deletions(-) diff --git a/awx/ui/src/components/RoutedTabs/RoutedTabs.js b/awx/ui/src/components/RoutedTabs/RoutedTabs.js index a3e559b4..7ae80830 100644 --- a/awx/ui/src/components/RoutedTabs/RoutedTabs.js +++ b/awx/ui/src/components/RoutedTabs/RoutedTabs.js @@ -38,13 +38,19 @@ function RoutedTabs({ tabsArray }) { const handleTabSelect = (event, eventKey) => { const match = tabsArray.find((tab) => tab.id === eventKey); - if (match) { - event.preventDefault(); - const link = match.persistentFilterKey - ? `${match.link}${getPersistentFilters(match.persistentFilterKey)}` - : match.link; - navigate(link); + // A tab can exist only to host a control in the tab bar, the workflow job + // selector being the one that does, and carries no link. Clicks inside such + // a control bubble up to the tab, so treating this as tab navigation would + // call navigate(undefined), land back on the current url, and undo whatever + // the control itself just did. + if (!match || !match.link) { + return; } + event.preventDefault(); + const link = match.persistentFilterKey + ? `${match.link}${getPersistentFilters(match.persistentFilterKey)}` + : match.link; + navigate(link); }; return ( {tab.name}} aria-controls="" ouiaId={`${tab.name}-tab`} diff --git a/awx/ui/src/components/RoutedTabs/RoutedTabs.test.js b/awx/ui/src/components/RoutedTabs/RoutedTabs.test.js index 047742ce..93b2b1fd 100644 --- a/awx/ui/src/components/RoutedTabs/RoutedTabs.test.js +++ b/awx/ui/src/components/RoutedTabs/RoutedTabs.test.js @@ -63,3 +63,53 @@ describe('', () => { ); }); }); + +describe(' with a tab that hosts a control', () => { + // The workflow job selector is registered as a tab with no link, so that it + // sits in the tab bar. Clicks on the control inside it bubble to the tab. + const controlTabs = [ + { name: 'Details', link: '/jobs/playbook/953/details', id: 0 }, + { name: 'Output', link: '/jobs/playbook/953/output', id: 1 }, + { + // a plain element, not a