ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
32 stars 8 forks source link

Address breadcrumb `aria-current` issues #2228

Open m-akinc opened 3 months ago

m-akinc commented 3 months ago

🧹 Tech Debt

FAST's breadcrumb has logic for updating aria-current on breadcrumb items, but there are multiple issues with it:

  1. The logic assumes that the last breadcrumb item (the one for the current page) has href='' set on it. The Nimble breadcrumb documentation and Storybook example do not state or follow that convention.

    • If href is not set, aria-current does not get set.
    • Note that the breadcrumb item template bakes in this assumption by not rendering a link when its href is ''. This is reasonable, given that href='' indicates the current page, and the breadcrumb item for the current page should render as non-interactive text.
  2. The code queries into the shadowRoots of child BreadcrumbItems.

  3. The code is only run when slotted items change. It does not respond to changes in those slotted breadcrumbs' href values or changes in their children,. We need to consider the implications and ensure all valid use patterns result in aria-current being set correctly.

  4. The logic implies that breadcrumb items support native a elements as content (as an alternative to setting an href directly on the breadcrumb item). Nimble breadcrumb items do not support that.