feat(router): add ariaCurrentWhenActive input to RouterLinkActive directive by dario-piotrowicz · Pull Request #45167 · angular/angular

dario-piotrowicz · 2022-02-22T22:55:36Z

add the ariaCurrentWhenActive input to the RouterLinkActive directive so that
users can easily set the aria-current property to their active router
links

resolves #35051

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: #35051

No build-in way to set the aria-current attribute to active routerLinks is present

What is the new behavior?

A new option has been added to the RouterLinkActive directive which sets the aria-current value when the link is active

Does this PR introduce a breaking change?

Yes
No

Other information

dario-piotrowicz · 2022-02-22T22:56:28Z

@atscott This PR is still in draft, but I was wondering if you'd like to have a quick initial look to see if I am going into the right direction 🙂

packages/router/src/directives/router_link_active.ts

dario-piotrowicz · 2022-02-23T21:11:12Z

TODOs:

mention the new input in: https://angular.io/guide/router-reference#active-router-links
mention the new input in: https://angular.io/guide/router-tutorial#identify-the-active-route
mention the new input in: https://angular.io/guide/cheatsheet
update the ToH examples with the new input (https://angular.io/guide/router-tutorial-toh) (all aio examples have been updated)
add a section about aria-current in the best practices accessibility guide

atscott

Some small comments but otherwise LGTM. Adding my approval so this can move forward to public API review.

aio/content/examples/animations/src/app/app.component.html

aio/content/guide/accessibility.md

aio/content/guide/router-tutorial.md

dario-piotrowicz · 2022-02-28T18:35:22Z

@atscott I'm so sorry one commit got somehow lost in my other notebook and what you reviewed was missing some changes, I have amended the thing (but had to rebase and squash the commits 😓), please have another look, so sorry for the inconvenience 😓

jessicajaniuk

LGTM 🍪

reviewed-for: fw-animations, public-api

dylhunn

reviewed-for: public-api

atscott

reviewed-for: public-api

packages/router/src/directives/router_link_active.ts

aio/content/guide/accessibility.md

packages/router/src/directives/router_link_active.ts

dylhunn · 2022-04-15T19:40:23Z

@atscott From reading the above thread, it sounds like we decided to stick with this approach? Please let me know if it needs to be merged (after the one docs comment is fixed), thanks :)

atscott · 2022-04-15T20:04:55Z

presubmit

…ective add the ariaCurrentWhenActive input to the RouterLinkActive directive so that users can easily set the aria-current property to their active router links resolves angular#35051

atscott · 2022-04-20T22:14:03Z

This PR was merged into the repository by commit dea8c86.

angular-automatic-lock-bot · 2022-05-21T16:07:00Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

dario-piotrowicz commented Feb 22, 2022

View reviewed changes

packages/router/src/directives/router_link_active.ts Outdated Show resolved Hide resolved

atscott requested changes Feb 22, 2022

View reviewed changes

dario-piotrowicz force-pushed the routerLink-active-aria-current branch from 0eac59f to 3d805c7 Compare February 23, 2022 20:59

dario-piotrowicz changed the title ~~feat(router): add ariaCurrent input to RouterLinkActive directive~~ feat(router): add ariaCurrentWhenActive input to RouterLinkActive directive Feb 23, 2022

dario-piotrowicz force-pushed the routerLink-active-aria-current branch 2 times, most recently from 48405eb to 5f87e2f Compare February 23, 2022 22:46

jessicajaniuk added the area: router label Feb 24, 2022

ngbot bot added this to the Backlog milestone Feb 24, 2022

dario-piotrowicz force-pushed the routerLink-active-aria-current branch 2 times, most recently from d8bfaa0 to 0eea389 Compare February 25, 2022 20:40

dario-piotrowicz marked this pull request as ready for review February 25, 2022 20:42

pullapprove bot requested a review from alxhub February 25, 2022 20:42

dario-piotrowicz force-pushed the routerLink-active-aria-current branch from 0eea389 to 21d0c88 Compare February 26, 2022 11:33

dario-piotrowicz requested a review from atscott February 27, 2022 19:10

atscott approved these changes Feb 28, 2022

View reviewed changes

aio/content/examples/animations/src/app/app.component.html Outdated Show resolved Hide resolved

aio/content/guide/accessibility.md Outdated Show resolved Hide resolved

aio/content/guide/router-tutorial.md Outdated Show resolved Hide resolved

atscott added the target: minor This PR is targeted for the next minor release label Feb 28, 2022

dario-piotrowicz force-pushed the routerLink-active-aria-current branch from bbe7380 to fff230d Compare February 28, 2022 18:34

jessicajaniuk approved these changes Feb 28, 2022

View reviewed changes

jessicajaniuk removed the request for review from alxhub February 28, 2022 18:44

pullapprove bot requested review from AndrewKushnir, atscott, dylhunn and jelbourn March 2, 2022 00:28

dylhunn approved these changes Mar 2, 2022

View reviewed changes

atscott approved these changes Mar 2, 2022

View reviewed changes

AndrewKushnir removed their request for review March 2, 2022 01:31

jelbourn approved these changes Mar 2, 2022

View reviewed changes

packages/router/src/directives/router_link_active.ts Outdated Show resolved Hide resolved

atscott added the action: discuss label Mar 3, 2022

dario-piotrowicz force-pushed the routerLink-active-aria-current branch 3 times, most recently from ec43678 to f552300 Compare April 9, 2022 13:11

jelbourn reviewed Apr 15, 2022

View reviewed changes

aio/content/guide/accessibility.md Outdated Show resolved Hide resolved

packages/router/src/directives/router_link_active.ts Outdated Show resolved Hide resolved

atscott force-pushed the routerLink-active-aria-current branch from f552300 to 7e9017e Compare April 15, 2022 20:01

atscott removed the action: discuss label Apr 15, 2022

atscott added the action: presubmit The PR is in need of a google3 presubmit label Apr 15, 2022

feat(router): add ariaCurrentWhenActive input to RouterLinkActive dir…

d8ab42d

…ective add the ariaCurrentWhenActive input to the RouterLinkActive directive so that users can easily set the aria-current property to their active router links resolves angular#35051

atscott added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Apr 20, 2022

atscott force-pushed the routerLink-active-aria-current branch from 7e9017e to d8ab42d Compare April 20, 2022 20:41

atscott closed this in dea8c86 Apr 20, 2022

dario-piotrowicz deleted the routerLink-active-aria-current branch April 20, 2022 22:20

dario-piotrowicz mentioned this pull request Apr 21, 2022

refactor(router): refactor RouterLinkActive to use host #45721

Closed

14 tasks

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 21, 2022

Conversation

dario-piotrowicz commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

dario-piotrowicz commented Feb 22, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dario-piotrowicz commented Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dario-piotrowicz commented Feb 28, 2022

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

dylhunn left a comment

Choose a reason for hiding this comment

Uh oh!

atscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dylhunn commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atscott commented Apr 15, 2022

Uh oh!

atscott commented Apr 20, 2022

Uh oh!

angular-automatic-lock-bot bot commented May 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dario-piotrowicz commented Feb 22, 2022 •

edited

Loading

dario-piotrowicz commented Feb 23, 2022 •

edited

Loading

dylhunn commented Apr 15, 2022 •

edited

Loading