Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc, tools: add ARIA landmarks to API docs #49882

Merged
merged 3 commits into from
Sep 28, 2023
Merged

doc, tools: add ARIA landmarks to API docs #49882

merged 3 commits into from
Sep 28, 2023

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 26, 2023

As an a11y enhancement, supply ARIA landmarks for all content.

@Trott Trott added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Sep 26, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/website

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Sep 26, 2023
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@Trott Trott marked this pull request as draft September 26, 2023 18:15
@Trott

This comment was marked as outdated.

Sign in to view
@Trott Trott marked this pull request as ready for review September 26, 2023 18:22
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54262/

@Trott
Copy link
Member Author

Trott commented Sep 26, 2023

@nodejs/nodejs-website

AugustinMauroy
AugustinMauroy reviewed Sep 26, 2023
View reviewed changes
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think use html semantic tags instead of role aria it's better and more logical.
But that's not really a problem.
REF:

@Trott
Copy link
Member Author

Trott commented Sep 26, 2023

I think use html semantic tags instead of role aria it's better and more logical. But that's not really a problem. REF:

* https://developer.mozilla.org/en-US/docs/Glossary/Semantics

* https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles

Sure, but that risks altering styling in unexpected ways and requiring more significant changes to html.mjs and/or allhtml.mjs. This is the minimal change. If you or someone else want to go in and try to update things to use semantic elements later, that's fine. But I won't be doing it in this PR. Small incremental changes FTW.

AugustinMauroy
AugustinMauroy approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 !

@ovflowd
Copy link
Member

ovflowd commented Sep 26, 2023

I think use html semantic tags instead of role aria it's better and more logical. But that's not really a problem. REF:

* https://developer.mozilla.org/en-US/docs/Glossary/Semantics

* https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles

Sure, but that risks altering styling in unexpected ways and requiring more significant changes to html.mjs and/or allhtml.mjs. This is the minimal change. If you or someone else want to go in and try to update things to use semantic elements later, that's fine. But I won't be doing it in this PR. Small incremental changes FTW.

Oh no, let's please not update this old hacky HTML template. Only lord knows what would break.

There's pending work for redesigning the API docs (based on the new Figma's and work of the Website Redesign) which will result on the tooling behind the API docs to get a refresh (not related to the API metadata proposal, but I want to tackle the current API tooling since a long time ago)

@Trott Trott mentioned this pull request Sep 26, 2023
Merged
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54299/

@github-actions github-actions bot mentioned this pull request Sep 28, 2023
Open
19 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54313/

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 28, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 28, 2023
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 28, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/49882
✔  Done loading data for nodejs/node/pull/49882
----------------------------------- PR info ------------------------------------
Title      doc, tools: add ARIA landmarks to API docs (#49882)
Author     Rich Trott  (@Trott)
Branch     Trott:docs-a11y -> nodejs:main
Labels     doc, tools, commit-queue-rebase
Commits    3
 - doc: add navigation ARIA landmark to doc ToC
 - doc: add main ARIA landmark to API docs
 - tools: add navigation ARIA landmark to generated API ToC
Committers 1
 - Rich Trott 
PR-URL: https://github.com/nodejs/node/pull/49882
Reviewed-By: Luigi Pinca 
Reviewed-By: Claudio Wunder 
Reviewed-By: LiviaMedeiros 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49882
Reviewed-By: Luigi Pinca 
Reviewed-By: Claudio Wunder 
Reviewed-By: LiviaMedeiros 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 26 Sep 2023 17:57:07 GMT
   ✔  Approvals: 3
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/49882#pullrequestreview-1644901184
   ✔  - Claudio Wunder (@ovflowd): https://github.com/nodejs/node/pull/49882#pullrequestreview-1645025930
   ✔  - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/49882#pullrequestreview-1645859889
   ✘  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6342605819

@Trott
doc: add navigation ARIA landmark to doc ToC
f416a0f 
As an accessibility improvement, specify the navigation landmark for
the column in our docs that contains the table of contents.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
@Trott
doc: add main ARIA landmark to API docs
bc056c2 
As an accessibility improvement, specify the main landmark for
the column in our docs that contains the main content.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
@Trott
tools: add navigation ARIA landmark to generated API ToC
7f06c27 
As an accessibility improvement, specify the navigation landmark for
the element of our docs that contains the table of contents generated
for the specific API page.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
@Trott Trott merged commit 7f06c27 into nodejs:main Sep 28, 2023
23 of 24 checks passed
@Trott
Copy link
Member Author

Trott commented Sep 28, 2023

Landed in e97d256...7f06c27

@Trott Trott deleted the docs-a11y branch September 28, 2023 21:10
@github-actions github-actions bot mentioned this pull request Sep 30, 2023
Open
31 tasks
@RafaelGSS RafaelGSS mentioned this pull request Oct 9, 2023
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@Trott @alexfernandez
doc: add navigation ARIA landmark to doc ToC
c49937d 
As an accessibility improvement, specify the navigation landmark for
the column in our docs that contains the table of contents.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@Trott @alexfernandez
doc: add main ARIA landmark to API docs
a86bd26 
As an accessibility improvement, specify the main landmark for
the column in our docs that contains the main content.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@Trott @alexfernandez
tools: add navigation ARIA landmark to generated API ToC
db3ca86 
As an accessibility improvement, specify the navigation landmark for
the element of our docs that contains the table of contents generated
for the specific API page.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
targos pushed a commit that referenced this pull request Nov 11, 2023
@Trott @targos
doc: add navigation ARIA landmark to doc ToC
959ef7a 
As an accessibility improvement, specify the navigation landmark for
the column in our docs that contains the table of contents.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: #49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
targos pushed a commit that referenced this pull request Nov 11, 2023
@Trott @targos
doc: add main ARIA landmark to API docs
337a676 
As an accessibility improvement, specify the main landmark for
the column in our docs that contains the main content.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: #49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
targos pushed a commit that referenced this pull request Nov 11, 2023
@Trott @targos
tools: add navigation ARIA landmark to generated API ToC
732b566 
As an accessibility improvement, specify the navigation landmark for
the element of our docs that contains the table of contents generated
for the specific API page.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: #49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
@targos targos mentioned this pull request Nov 12, 2023
Merged
@juanarbol juanarbol mentioned this pull request Dec 6, 2023
Merged
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@Trott @debadree25
doc: add navigation ARIA landmark to doc ToC
c0826c7 
As an accessibility improvement, specify the navigation landmark for
the column in our docs that contains the table of contents.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@Trott @debadree25
doc: add main ARIA landmark to API docs
af6db5c 
As an accessibility improvement, specify the main landmark for
the column in our docs that contains the main content.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
@Trott @debadree25
tools: add navigation ARIA landmark to generated API ToC
92295eb 
As an accessibility improvement, specify the navigation landmark for
the element of our docs that contains the table of contents generated
for the specific API page.

Ref: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
Ref: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA20.html
PR-URL: nodejs#49882
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AugustinMauroy AugustinMauroy AugustinMauroy approved these changes

@LiviaMedeiros LiviaMedeiros LiviaMedeiros approved these changes

@mikeesto mikeesto mikeesto approved these changes

@aymen94 aymen94 aymen94 approved these changes

@ovflowd ovflowd ovflowd approved these changes

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Trott @nodejs-github-bot @ovflowd @lpinca @aymen94 @mikeesto @LiviaMedeiros @AugustinMauroy