Refactor TopAppBarLabel into Breadcrumbs
- Merged
TopAppBarLabel
intoBreadcrumbs
component (previouslySubgraphBreadcrumbs
). Reworked a bit the styling. - Reworked
displayedWorkflowHierarchy
to only store parents (or ancestors), following the recommendation in !390 (comment 280913) - Fixed the Cypress tests to match the new
Breadcrumbs
component structure
Merge request reports
Activity
- Resolved by Loic Huder
- Resolved by Loic Huder
9 10 ) { 10 const workflowIndexInHierarchy: number = oldHierarchy.indexOf( 11 newWorkflowInfo.id 12 ); 13 14 if (workflowIndexInHierarchy === -1) { 15 return [...oldHierarchy, newWorkflowInfo.id]; 11 if (prevWorkflowId === '') { 12 return prevParents; 16 13 } 14 const nextWorkflowIsAParent = new Set(prevParents).has(nextWorkflowId); 17 15 18 if (workflowIndexInHierarchy === oldHierarchy.length - 1) { 19 // TODO: if user insert the same 'graph' and is the first then stack is not updated 20 // Not applicable so left as is and it just wont be able to doubleClick 21 return oldHierarchy; 3 3 import type { SetState } from 'zustand'; 4 4 import { merge } from 'lodash'; 5 5 6 function getNewHierarchy( 7 newWorkflowInfo: GraphDetails, 8 oldHierarchy: string[] 6 function getParentsOfNextWorkflow( 7 nextWorkflowId: string, 8 prevWorkflowId: string, 5 5 6 function getNewHierarchy( 7 newWorkflowInfo: GraphDetails, 8 oldHierarchy: string[] 6 function getParentsOfNextWorkflow( 7 nextWorkflowId: string, 8 prevWorkflowId: string, 9 prevParents: string[] 9 10 ) { 10 const workflowIndexInHierarchy: number = oldHierarchy.indexOf( 11 newWorkflowInfo.id 12 ); 13 14 if (workflowIndexInHierarchy === -1) { 15 return [...oldHierarchy, newWorkflowInfo.id]; 11 if (prevWorkflowId === '') { 94 cy.findByRole('button', { name: 'Open edit actions menu' }).click(); 95 cy.findByRole('menuitem', { name: 'Clone Workflow' }).click(); 104 96 cy.waitForStableDOM(); 105 97 106 cy.contains('Give the new workflow identifier') 107 .parent() 108 .should('have.class', 'MuiDialogTitle-root') 109 .siblings() 110 .first() 111 .as('dialogContent') 112 .should('have.class', 'MuiDialogContent-root'); 98 cy.findByRole('dialog').within(() => { 99 cy.findAllByRole('heading', { 100 name: 'Give the new workflow identifier', 101 }).should('be.visible'); 102 }); assigned to @bocciare
requested review from @bocciare
unassigned @bocciare
49 53 fitView({ duration: 500 }); 50 54 }, 300); 51 55 } 52 }; 53 56 54 57 return ( 55 <Breadcrumbs aria-label="breadcrumb" color="inherit"> 56 {subgraphsStack.map((graphId, index) => { 57 const graphLabel = loadedGraphs.get(graphId)?.graph.label || graphId; 58 <MuiBreadcrumbs 59 className={styles.title} 60 aria-label="breadcrumb" Found this: https://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-breadcrumbs in case you want to double check the markup (
nav
element, pluralised label: "breadcrumbs",aria-current
). That said, maybe best to not use thenav
element, since we're already inside anh1
.Ha! I was actually following the guidelines at https://www.w3.org/WAI/ARIA/apg/patterns/breadcrumb/examples/breadcrumb/ and https://www.w3.org/WAI/ARIA/apg/patterns/breadcrumb/. These state that "breadcrumb" is single (though I find the plural more logical) and that
aria-current
is optional if the last crumb is not a link (which is the case here).No strong opinions through, I am happy to go with the most sensible recommendation.
24 30 const setDataFromEdges = useEdgeDataStore((state) => state.setDataFromEdges); 25 31 26 if (subgraphsStack.length === 1) { 27 const { id, label = id } = displayedWorkflowInfo; 28 return <span data-cy={label}>{label}</span>; 29 } 30 31 const goToGraph = (e: React.MouseEvent) => { 32 e.preventDefault(); 32 const rootWorkflowId = useStore((state) => state.rootWorkflowId); 33 33 34 const { target } = e; 34 if (!rootWorkflowId) { 35 return ( 36 <> 37 Untitled_workflow <span className={styles.labelHint}>(unsaved)</span> changed this line in version 3 of the diff
8 oldHierarchy: string[] 6 function getParentsOfNextWorkflow( 7 nextWorkflowId: string, 8 prevWorkflowId: string, 9 prevParents: string[] 9 10 ) { 10 const workflowIndexInHierarchy: number = oldHierarchy.indexOf( 11 newWorkflowInfo.id 12 ); 13 14 if (workflowIndexInHierarchy === -1) { 15 return [...oldHierarchy, newWorkflowInfo.id]; 11 if (prevWorkflowId === '') { 12 return prevParents; 16 13 } 14 const nextWorkflowIsAParent = new Set(prevParents).has(nextWorkflowId); changed this line in version 3 of the diff
added 2 commits
mentioned in commit c46be140