fix(breadcrumb): expose aria-current on the current page item - #1402
Open
dmorfav wants to merge 1 commit into
Open
fix(breadcrumb): expose aria-current on the current page item#1402dmorfav wants to merge 1 commit into
dmorfav wants to merge 1 commit into
Conversation
The current item now sets aria-current="page", as required by the APG breadcrumb pattern. Items hidden through visible: false are skipped so the attribute lands on the item the user actually sees. Items with routerLink go through the ariaCurrentWhenActive input rather than an attribute binding: RouterLinkActive removes aria-current on every update, after the binding has run. Fixes openng-org#54
dmorfav
force-pushed
the
fix/breadcrumb-aria-current
branch
from
July 27, 2026 06:50
6501891 to
127880a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #54
p-breadcrumbnever exposed the current page to assistive technology, even though the component's own Accessibility docs already promise it:The current item now actually does that, as required by the APG breadcrumb pattern.
Two link branches, two mechanisms
The template renders items through two different anchors, and they need different handling:
urlitems —[attr.aria-current], following the convention already used bygalleria,carousel,steps,paginatorandstepper.routerLinkitems —[ariaCurrentWhenActive], theRouterLinkActiveinput. An attribute binding does not survive here:RouterLinkActive.update()callsremoveAttribute('aria-current')on every update, inside aqueueMicrotask, so it always runs after the binding and wipes it.That second point confirms the report in the issue — the passthrough on
itemLinkreally was being overwritten. The cause isRouterLinkActive, notrouterLinkitself.This also means
steps.ts:55's[attr.ariaCurrentWhenActive]is a no-op: with theattr.prefix it renders a literal attribute instead of binding the directive input. Left untouched here, but worth a separate look.Behaviour
Items hidden with
visible: falseare skipped, so the attribute lands on the item the user actually sees instead of on an<li>that is never rendered.Note the resulting asymmetry:
urlitems always mark the last visible one, whilerouterLinkitems are only marked when the router considers them active. In a real breadcrumb both coincide, since the last item is where you are. I kept the router-driven behaviour rather than forcing the attribute, because claimingaria-current="page"on a link pointing somewhere else would be worse than not setting it. Happy to change it if you'd rather have it unconditional.Scope
Only items in
model. Thehomelink lives in a separate branch of the template and is left untouched, so with an emptymodelnothing is marked. Tell me if you wanthometo be current when it is the only entry.Tests
Four cases in the existing accessibility block of
breadcrumb.spec.ts, asserting on the rendered DOM rather than on component state: the lasturlitem is marked and the preceding ones carry noaria-currentat all; hidden trailing items are skipped; arouterLinkitem is marked once its route is active, and not before. Full breadcrumb suite: 89 passing.Beyond Karma, I went through the docs app and checked in the rendered DOM: no regression on the Template demo (which renders no anchors of its own),
routerLinkActivestill applies its CSS class, no new console warnings, the attribute follows the last item whenmodelis replaced, and empty / single-item / all-hidden /undefinedmodels all behave without throwing.VoiceOver on Safari announces the last item as "Wireless, current page, list item", which is the behaviour the issue asks for.
I also ran the Lighthouse/axe audit on
/breadcrumbfrom bothmainand this branch: same score (80), same 8 failures, identical rule-and-selector list — no new violation.aria-valid-attr-value,aria-allowed-attrandaria-conditional-attrare all evaluated against the renderedaria-currentand pass.One pre-existing violation worth flagging, untouched here: axe reports
link-nameon thehomeanchor, because the demo declares it as{ icon: 'pi pi-home' }with no label and nohomeAriaLabel, leaving the link with no accessible name. Present onmaintoo. Happy to open a separate issue for it.One thing worth knowing: passthrough still cannot drive this attribute, on either branch.
pt: { itemLink: { 'aria-current': 'step' } }is overridden — by the binding onurlitems, and byRouterLinkActiveonrouterLinkones. Not a regression (it never worked, which is what the issue reported), but it does mean values other thanpagearen't reachable. Say the word if you'd like that configurable.Also spotted while testing, unrelated and left alone: with
model = []and ahomeset, the separator atbreadcrumb.ts:83still renders —*ngIf="model && home", and an empty array is truthy — leaving a trailing chevron with nothing after it. Happy to file it separately.