fix(docs-infra): apply various a11y aio improvements by dario-piotrowicz · Pull Request #45209 · angular/angular

dario-piotrowicz · 2022-02-26T18:45:12Z

changes:

currently the navigation ul used in aio-top-menu has a role of
navigation, but listitems should be owned by list parents
(see more: https://www.w3.org/TR/wai-aria-1.1/#listitem)
so wrap the ul in a nav and remove the role="navigation" from the
ul element to fix such issue
wrap the main aio mat-toolbar in a header element to provide better
accessibility
remove reduntant role="main"
(try to) make sure that there in only a single main in the aio page
(try to) make sure all elements are included in landmarks
(try to) make sure the main navs have different labels

resolves #16938
resolves #44562

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:

Does this PR introduce a breaking change?

Yes
No

jessicajaniuk

LGTM 🍪

I love seeing this a11y work, @dario-piotrowicz. Keep up the great work!

dario-piotrowicz · 2022-03-14T19:55:06Z

@gkalpak could you have a quick look at this one please? 😊

mary-poppins · 2022-03-15T20:22:29Z

You can preview 2e2280f at https://pr45209-2e2280f.ngbuilds.io/.
You can preview 603c996 at https://pr45209-603c996.ngbuilds.io/.
You can preview 635d4b3 at https://pr45209-635d4b3.ngbuilds.io/.
You can preview 3233964 at https://pr45209-3233964.ngbuilds.io/.

gkalpak

Great work, @dario-piotrowicz 👏

BTW, in the future, it is a good idea to break up such changes into separate commits 😉

gkalpak · 2022-03-15T20:34:41Z

While this increases the contrast between the anchor text and the background, it makes it harder to distinguish the anchor text from the regular text, which is not good a11y/UX either. Could we maybe make this a little lighter (and still meet contrast requirements)?

I'll have a look 🙂👍

I've updated it to #055cff (please let me know if you mind the hex there instead of a constant 😓)

This is how it looked before:

and this is after:

we do get the AA check

what do you think? 🙂
is the blue a bit too vibrant? 😓

It is a bit too vibrant for my taste. It feels a lit out of place with the test of our color-palette 😞
(We could really use a designer 👀 😛)

dario-piotrowicz · 2022-03-15T20:46:18Z

@gkalpak thanks a lot for the speedy review! ❤️ 😍 ❤️ 😍 ❤️ 😍 ❤️ 😍

dario-piotrowicz · 2022-03-15T20:57:38Z

Great work, @dario-piotrowicz clap

BTW, in the future, it is a good idea to break up such changes into separate commits wink

About the commits, yeah sorry I did started that way but then things were intertwining and I just figured I could just mash it into a single one 😅, as part of my next changes to this PR I can put the effort and try to divide stuff in ways that make sense, I am totally fine with that, it should not be too much of an issue if you want 🙂

(also the commits would be a bit small, hopefully you don't mind that 😄)

gkalpak · 2022-03-16T11:40:20Z

as part of my next changes to this PR I can put the effort and try to divide stuff in ways that make sense

No, no need for that. Just something to keep in mind for future PRs 😉

also the commits would be a bit small, hopefully you don't mind that 😄

We certainly don't mind small commits (as long as they are self-contained) 😃

dario-piotrowicz · 2022-03-16T22:16:50Z

@gkalpak I've addressed your comments, please have another look 😊

mary-poppins · 2022-03-22T20:09:53Z

You can preview 718f709 at https://pr45209-718f709.ngbuilds.io/.

gkalpak · 2022-03-23T18:17:04Z

Note: If we decide to keep this (or any color we end up with), we need to make sure that links inside code-blocks have the same color (and the color has to be accessible with both the default white-ish background and the gray code-block background).

currently the navigation ul used in aio-top-menu has a role of navigation, but listitems should be owned by list parents (see more: https://www.w3.org/TR/wai-aria-1.1/#listitem) so wrap the ul in a nav and remove the role="navigation" from the ul element to fix such issue resolves angular#44562 resolves angular#16938 (the second point)

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves angular#16938 (the first point)

remove redundant main role as pages should always have a single element with a main role (also remove the role assigne to the main tag as that is implied)

assign different aria labels to the primary nav and the one used for guides and docs, so that impaired users can more easily distinguish the two

mary-poppins · 2022-03-28T19:46:58Z

You can preview 7409b7e at https://pr45209-7409b7e.ngbuilds.io/.

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes

dario-piotrowicz · 2022-03-28T20:28:01Z

@gkalpak I've removed the color changes as those seems to be a bit tricky and could be handled separately, please have another look 🙂

(also now unfortunately the a11y scores are now going slightly down instead of going up 😭)

mary-poppins · 2022-03-28T20:31:32Z

You can preview 8077771 at https://pr45209-8077771.ngbuilds.io/.

gkalpak

dylhunn · 2022-03-29T16:23:16Z

This PR was merged into the repository by commit 16e635c.

currently the navigation ul used in aio-top-menu has a role of navigation, but listitems should be owned by list parents (see more: https://www.w3.org/TR/wai-aria-1.1/#listitem) so wrap the ul in a nav and remove the role="navigation" from the ul element to fix such issue resolves #44562 resolves #16938 (the second point) PR Close #45209

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves #16938 (the first point) PR Close #45209

remove redundant main role as pages should always have a single element with a main role (also remove the role assigne to the main tag as that is implied) PR Close #45209

assign different aria labels to the primary nav and the one used for guides and docs, so that impaired users can more easily distinguish the two PR Close #45209

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes PR Close #45209

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves #16938 (the first point) PR Close #45209

remove redundant main role as pages should always have a single element with a main role (also remove the role assigne to the main tag as that is implied) PR Close #45209

assign different aria labels to the primary nav and the one used for guides and docs, so that impaired users can more easily distinguish the two PR Close #45209

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes PR Close #45209

angular-automatic-lock-bot · 2022-05-10T16:15:12Z

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.}

pullapprove Bot requested review from alxhub and jessicajaniuk February 26, 2022 18:45

dario-piotrowicz force-pushed the aio-a11y-improvements branch 3 times, most recently from 635d4b3 to 3233964 Compare February 27, 2022 13:12

jessicajaniuk added comp: docs labels Feb 28, 2022

ngbot Bot modified the milestone: Backlog Feb 28, 2022

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

jessicajaniuk approved these changes Feb 28, 2022

View reviewed changes

gkalpak added type: bug/fix aio: preview and removed comp: docs labels Mar 15, 2022

gkalpak suggested changes Mar 15, 2022

View reviewed changes

dario-piotrowicz force-pushed the aio-a11y-improvements branch from 3233964 to 64f091f Compare March 16, 2022 21:53

dario-piotrowicz requested a review from gkalpak March 16, 2022 22:14

dario-piotrowicz force-pushed the aio-a11y-improvements branch 2 times, most recently from 5c53094 to 718f709 Compare March 22, 2022 19:59

gkalpak reviewed Mar 23, 2022

View reviewed changes

dario-piotrowicz added 3 commits March 28, 2022 20:30

fix(docs-infra): wrap the main aio mat-toolbar in a header

a7a4e69

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves angular#16938 (the first point)

fix(docs-infra): remove redundant main roles

62c9ec0

remove redundant main role as pages should always have a single element with a main role (also remove the role assigne to the main tag as that is implied)

fix(docs-infra): assign different aria labels to main aio navs

7b0e45c

assign different aria labels to the primary nav and the one used for guides and docs, so that impaired users can more easily distinguish the two

dario-piotrowicz force-pushed the aio-a11y-improvements branch from 718f709 to 7409b7e Compare March 28, 2022 19:37

dario-piotrowicz force-pushed the aio-a11y-improvements branch from 7409b7e to 1bb321e Compare March 28, 2022 20:20

fix(docs-infra): update a11y min-scores

8077771

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes

dario-piotrowicz force-pushed the aio-a11y-improvements branch from 1bb321e to 8077771 Compare March 28, 2022 20:21

dario-piotrowicz commented Mar 28, 2022

View reviewed changes

Comment thread aio/scripts/test-aio-a11y.mjs

dario-piotrowicz requested a review from gkalpak March 28, 2022 20:26

gkalpak approved these changes Mar 29, 2022

View reviewed changes

gkalpak removed the request for review from alxhub March 29, 2022 10:17

gkalpak added the action: merge The PR is ready for merge by the caretaker label Mar 29, 2022

dylhunn pushed a commit that referenced this pull request Mar 29, 2022

fix(docs-infra): wrap the main aio mat-toolbar in a header (#45209)

14cabb0

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves #16938 (the first point) PR Close #45209

dylhunn pushed a commit that referenced this pull request Mar 29, 2022

fix(docs-infra): update a11y min-scores (#45209)

7bbf009

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes PR Close #45209

dylhunn closed this in 7ef0717 Mar 29, 2022

dylhunn pushed a commit that referenced this pull request Mar 29, 2022

fix(docs-infra): wrap the main aio mat-toolbar in a header (#45209)

be5dce2

wrap the main aio mat-toolbar in a header element to provide better accessibility resolves #16938 (the first point) PR Close #45209

dylhunn pushed a commit that referenced this pull request Mar 29, 2022

fix(docs-infra): update a11y min-scores (#45209)

16e635c

update (decrease) the value of some of the accessibility MIN_SCORES_PER_PAGE after recent changes PR Close #45209

dario-piotrowicz deleted the aio-a11y-improvements branch April 9, 2022 12:30

dario-piotrowicz mentioned this pull request Apr 24, 2022

AIO header navigation bar ARIA and screen-reader support #44335

Closed

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

Conversation

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

Uh oh!

PR Checklist

PR Type

Does this PR introduce a breaking change?

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz commented Mar 14, 2022

Uh oh!

mary-poppins commented Mar 15, 2022

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gkalpak Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz Mar 16, 2022

Choose a reason for hiding this comment

Uh oh!

gkalpak Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

dario-piotrowicz commented Mar 15, 2022

Uh oh!

dario-piotrowicz commented Mar 15, 2022

Uh oh!

gkalpak commented Mar 16, 2022

Uh oh!

dario-piotrowicz commented Mar 16, 2022

Uh oh!

mary-poppins commented Mar 22, 2022

Uh oh!

gkalpak Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Mar 28, 2022

Uh oh!

Uh oh!

dario-piotrowicz commented Mar 28, 2022

Uh oh!

mary-poppins commented Mar 28, 2022

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

dylhunn commented Mar 29, 2022

Uh oh!

angular-automatic-lock-bot Bot commented May 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dario-piotrowicz commented Feb 26, 2022 •

edited

Loading