remove href on crumbs with aria-disabled="true"

[jorycunningham]

Description

Removed href on last breadcrump in a group so that we can use aria-disabled on it while retaining validity of the HTML.

How has this been tested?

• Ran axe-core against build
• Validated rendered HTML in W3C Nu Validator
• Inspected rendered DOM

Documentation changes

[Do the changes include any API documentation changes?]

• [ ] Yes, this change contains documentation changes.
• [X] No.

Related Links

AWSUI-18939

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

• [X] Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
• [X] Changes do not include unsupported browser features, see CONTRIBUTING.md.
• [X] Changes were manually tested for accessibility, see accessibility guidelines.

Security

• [ ] If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

• [ ] Changes are covered with new/existing unit tests?
• [ ] Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[jorycunningham]

PR title is a bit misleading and does not represent what the code is technically doing. The code is removing the href from the last breadcrumb which just happens to be the disabled.

Apologies, I'm not sure I follow this. The reason we are removing the href IS that the last one has aria-disabled, it doesn't just happen to be disabled. The recommended resolution for AWSUI-18939 is to remove href on crumbs with aria-disabled="true" to prevent invalid HTML from the combination of the two attributes. Were aria-disabled not present there would be no need to remove the href.

[michaeldowseza]

Apologies, I'm not sure I follow this. The reason we are removing the href IS that the last one has aria-disabled, it doesn't just happen to be disabled. The recommended resolution for AWSUI-18939 is to remove href on crumbs with aria-disabled="true" to prevent invalid HTML from the combination of the two attributes. Were aria-disabled not present there would be no need to remove the href.

The misleading part is the title currently suggests that any breadcrumb that it will remove the href of any breadcrumb that has aria-disabled=true but that is not the case.

[jorycunningham]

I see, thank you for the explanation @michaeldowseza . I've updated the PR title and unit test logic.

[codecov[bot]]

Codecov Report

Merging #117 (9f376d3) into main (8f12e74) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   92.38%   92.39%   +0.01%     
==========================================
  Files         535      536       +1     
  Lines       15462    15485      +23     
  Branches     4254     4256       +2     
==========================================
+ Hits        14284    14307      +23     
- Misses       1095     1096       +1     
+ Partials       83       82       -1     

Impacted Files	Coverage Δ
src/breadcrumb-group/item/item.tsx	100.00% <100.00%> (ø)
src/autosuggest/internal.tsx	93.45% <0.00%> (-0.61%)	:arrow_down:
src/autosuggest/plain-list.tsx	100.00% <0.00%> (ø)
src/date-picker/calendar/index.tsx	100.00% <0.00%> (ø)
src/date-range-picker/calendar/index.tsx	79.84% <0.00%> (ø)
src/date-range-picker/calendar/header/index.tsx	100.00% <0.00%> (ø)
...date-range-picker/calendar/header/button/index.tsx	100.00% <0.00%> (ø)
src/autosuggest/hooks/a11y.ts
src/autosuggest/options-list.tsx	97.14% <0.00%> (ø)
src/date-picker/use-date-picker.tsx	95.23% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.