From dfdedd5da09d6ecea9ea5966c416e91663ed5ddd Mon Sep 17 00:00:00 2001 From: Amr Mohamed Date: Mon, 10 Feb 2025 10:33:43 +0200 Subject: [PATCH 1/3] chore: Add proper textual name, role and state to breadcrumb last item --- src/breadcrumb-group/item/item.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/breadcrumb-group/item/item.tsx b/src/breadcrumb-group/item/item.tsx index f159fb277f..0bd662d7af 100644 --- a/src/breadcrumb-group/item/item.tsx +++ b/src/breadcrumb-group/item/item.tsx @@ -101,11 +101,18 @@ export function BreadcrumbItem({ const anchorAttributes: React.AnchorHTMLAttributes = { href: item.href || '#', onClick: isLast ? preventDefault : onClickHandler, + tabIndex: 0, }; if (isGhost) { anchorAttributes.tabIndex = -1; } + if (isLast) { + anchorAttributes.role = 'link'; + anchorAttributes['aria-disabled'] = true; + anchorAttributes['aria-current'] = 'page'; + } + const breadcrumbItem = ( Date: Mon, 10 Feb 2025 10:35:40 +0200 Subject: [PATCH 2/3] chore: Adjust logic for last item and add tests --- .../__integ__/breadcrumb-group.test.ts | 36 +++++++++---------- .../__tests__/breadcrumb-item.test.tsx | 8 ++++- src/breadcrumb-group/item/item.tsx | 25 +++++++------ 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/breadcrumb-group/__integ__/breadcrumb-group.test.ts b/src/breadcrumb-group/__integ__/breadcrumb-group.test.ts index 676fa398b1..7a6b0c9296 100644 --- a/src/breadcrumb-group/__integ__/breadcrumb-group.test.ts +++ b/src/breadcrumb-group/__integ__/breadcrumb-group.test.ts @@ -169,23 +169,23 @@ describe('BreadcrumbGroup', () => { ) ); - test( - 'Last item is focusable when truncated', - setupTest( - async page => { - await page.click('#focus-target-long-text'); - await page.keys('Tab'); - await page.keys('Tab'); - await page.keys('Tab'); - await page.keys('Tab'); - await page.keys('Tab'); - await page.keys('Tab'); - await expect(page.isTooltipDisplayed()).resolves.toBe(true); - await page.keys('Tab'); - await expect(page.isTooltipDisplayed()).resolves.toBe(false); - await expect(page.getActiveElementId()).resolves.toBe('focus-target-short-text'); - }, - { width: 950, height: 800 } - ) + test.each([ + [{ width: 770, height: 800 }, 'when truncated'], + [{ width: 1200, height: 800 }, 'default state'], + // eslint-disable-next-line @typescript-eslint/no-unused-vars + ])('Last item should be focusable %s', (viewportSize, scenario) => + setupTest(async page => { + await page.click('#focus-target-long-text'); + await page.keys('Tab'); + await page.keys('Tab'); + await page.keys('Tab'); + await page.keys('Tab'); + await page.keys('Tab'); + await page.keys('Tab'); + await expect(page.isTooltipDisplayed()).resolves.toBe(true); + await page.keys('Tab'); + await expect(page.isTooltipDisplayed()).resolves.toBe(false); + await expect(page.getActiveElementId()).resolves.toBe('focus-target-short-text'); + }, viewportSize)() ); }); diff --git a/src/breadcrumb-group/__tests__/breadcrumb-item.test.tsx b/src/breadcrumb-group/__tests__/breadcrumb-item.test.tsx index dcfb67d976..3cf33617e1 100644 --- a/src/breadcrumb-group/__tests__/breadcrumb-item.test.tsx +++ b/src/breadcrumb-group/__tests__/breadcrumb-item.test.tsx @@ -114,10 +114,16 @@ describe('BreadcrumbGroup Item', () => { test('should not be a link', () => { expect(lastLink.getElement()).not.toHaveAttribute('href'); - expect(lastLink.getElement()).not.toHaveAttribute('aria-current'); expect(lastLink.getElement().tagName).toEqual('SPAN'); }); + test('should have proper aria-labels and focusable', () => { + expect(lastLink.getElement()).toHaveAttribute('aria-current'); + expect(lastLink.getElement()).toHaveAttribute('aria-disabled'); + expect(lastLink.getElement().getAttribute('role')).toBe('link'); + expect(lastLink.getElement().getAttribute('tabindex')).toBe('0'); + }); + test('should not trigger click event', () => { lastLink.click(); expect(onClickSpy).not.toHaveBeenCalled(); diff --git a/src/breadcrumb-group/item/item.tsx b/src/breadcrumb-group/item/item.tsx index 0bd662d7af..7a94138e7c 100644 --- a/src/breadcrumb-group/item/item.tsx +++ b/src/breadcrumb-group/item/item.tsx @@ -54,7 +54,6 @@ const BreadcrumbItemWithPopover = ({ }} onMouseLeave={() => setShowPopover(false)} anchorAttributes={anchorAttributes} - {...(isLast ? { tabIndex: 0 } : {})} > {children} @@ -68,16 +67,25 @@ type ItemProps = React.HTMLAttributes & { isLast: boolean; }; const Item = React.forwardRef( - ({ anchorAttributes, children, isLast, ...itemAttributes }, ref) => - isLast ? ( - + ({ anchorAttributes, children, isLast, ...itemAttributes }, ref) => { + return isLast ? ( + {children} ) : ( } className={styles.anchor} {...itemAttributes} {...anchorAttributes}> {children} - ) + ); + } ); export function BreadcrumbItem({ @@ -103,16 +111,11 @@ export function BreadcrumbItem({ onClick: isLast ? preventDefault : onClickHandler, tabIndex: 0, }; + if (isGhost) { anchorAttributes.tabIndex = -1; } - if (isLast) { - anchorAttributes.role = 'link'; - anchorAttributes['aria-disabled'] = true; - anchorAttributes['aria-current'] = 'page'; - } - const breadcrumbItem = ( Date: Mon, 10 Feb 2025 11:19:20 +0200 Subject: [PATCH 3/3] chore: Distinguish between ghost and normal item --- src/breadcrumb-group/item/item.tsx | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/breadcrumb-group/item/item.tsx b/src/breadcrumb-group/item/item.tsx index 7a94138e7c..09b1d12089 100644 --- a/src/breadcrumb-group/item/item.tsx +++ b/src/breadcrumb-group/item/item.tsx @@ -18,12 +18,14 @@ interface BreadcrumbItemWithPopoverProps { isLast: boolean; anchorAttributes: React.AnchorHTMLAttributes; children: React.ReactNode; + itemAttributes: React.HTMLAttributes; } const BreadcrumbItemWithPopover = ({ item, isLast, anchorAttributes, + itemAttributes, children, }: BreadcrumbItemWithPopoverProps) => { const [showPopover, setShowPopover] = useState(false); @@ -54,6 +56,7 @@ const BreadcrumbItemWithPopover = ({ }} onMouseLeave={() => setShowPopover(false)} anchorAttributes={anchorAttributes} + {...itemAttributes} > {children} @@ -69,15 +72,7 @@ type ItemProps = React.HTMLAttributes & { const Item = React.forwardRef( ({ anchorAttributes, children, isLast, ...itemAttributes }, ref) => { return isLast ? ( - + {children} ) : ( @@ -112,10 +107,18 @@ export function BreadcrumbItem({ tabIndex: 0, }; + const itemAttributes: React.AnchorHTMLAttributes = {}; if (isGhost) { anchorAttributes.tabIndex = -1; } + if (isLast && !isGhost) { + itemAttributes['aria-current'] = 'page'; + itemAttributes['aria-disabled'] = true; + itemAttributes.tabIndex = 0; + itemAttributes.role = 'link'; + } + const breadcrumbItem = ( ({ return (
{isTruncated && !isGhost ? ( - + {breadcrumbItem} ) : ( - + {breadcrumbItem} )}