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.

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

docs: update aria labelling for sidebar toggles and toctree groups #7381

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

ashtonG
Copy link
Contributor

@ashtonG ashtonG commented Jul 13, 2023

Description

This updates the docs to fix the accessibility issues highlighted in the levelaccess tool.

  • The sidebar toggles as well as the labels controlling the expansion and collapse of the table of contents have been given screenreader labels
  • the inputs controlling the sidebar visibility have been updated to indicate that they control the primary/secondary sidebars as well as a pointer to a descriptive label

In order to do this properly, we fork the pydata-sphinx-theme python package and make the changes there. The changes to the package can be seen in determined-ai/pydata-sphinx-theme#1 -- once approved, we can change the branch this PR is pointing to to main.

Test Plan

Docs site should not have any 10 severity accessibility issues in levelaccess.

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

WEB-1292

@ashtonG ashtonG requested a review from a team as a code owner July 13, 2023 17:44
@cla-bot cla-bot bot added the cla-signed label Jul 13, 2023
@netlify
Copy link

netlify bot commented Jul 13, 2023

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit bfa9592
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/64baaf6b8c0a9b0008f78b60

@determined-ci
Copy link
Collaborator

Docsite preview being generated for this PR.
You can (eventually) find the generated docsite here.

@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jul 13, 2023
@ashtonG ashtonG requested review from a team, thiagodallacqua-hpe and hkang1 July 13, 2023 20:58
@hkang1 hkang1 self-assigned this Jul 19, 2023
Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

I did a make clean && make get-deps && make build && make live and it looks like it is building and serving without a hitch. But when I look at the targeted toctree-checkbox areas based on the report, they haven't changed as shown in the screenshot below. Maybe the changes are not coming through?

Screenshot 2023-07-19 at 10 08 07 AM

For the toctree-checkbox issues, they are basically invisible checkboxes to keep track of collapsible state of the sidebar menu. It might be best to apply the following attribute instead?

aria-hidden="true"

UPDATE: I'm not seeing your changes in the forked branch in the new build. What do you typically do for your local build? It seems like it's pulling in the right dependency but the changes in the branch don't seem to be in the newly built doc...

❯ make get-deps
pip install -r requirements.txt
Collecting pydata-sphinx-theme@ git+https://github.com/determined-ai/pydata-sphinx-theme@chore/WEB-1292/hpe-accessibility-patches
  Cloning https://github.com/determined-ai/pydata-sphinx-theme (to revision chore/WEB-1292/hpe-accessibility-patches) to /private/var/folders/ws/1f9qnqy5543c1ymspl1bkgnr0000gp/T/pip-install-8sto9o42/pydata-sphinx-theme_ff07c9aff8ff479f9604f1dc1b0e6e05
  Running command git clone --filter=blob:none --quiet https://github.com/determined-ai/pydata-sphinx-theme /private/var/folders/ws/1f9qnqy5543c1ymspl1bkgnr0000gp/T/pip-install-8sto9o42/pydata-sphinx-theme_ff07c9aff8ff479f9604f1dc1b0e6e05
  Running command git checkout -b chore/WEB-1292/hpe-accessibility-patches --track origin/chore/WEB-1292/hpe-accessibility-patches
  Switched to a new branch 'chore/WEB-1292/hpe-accessibility-patches'
  branch 'chore/WEB-1292/hpe-accessibility-patches' set up to track 'origin/chore/WEB-1292/hpe-accessibility-patches'.
  Resolved https://github.com/determined-ai/pydata-sphinx-theme to commit e3a365c78e79f10923d3a46c001fa3bef8833550
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done

@hkang1 hkang1 assigned ashtonG and unassigned hkang1 Jul 19, 2023
@ashtonG
Copy link
Contributor Author

ashtonG commented Jul 19, 2023

i run with make -C docs get-deps and then make -C docs clean live. I'm wondering if there's something cached locally somewhere, because the docs changes are being picked up on the docs preview:
image

I did have to run make -C examples once in order to get the build to succeed?

I don't think aria-hidden=true is the right approach for these checkboxes because they aren't decorative -- the user interacts with the inputs via the labels in the html.

@ashtonG ashtonG assigned hkang1 and unassigned ashtonG Jul 19, 2023
Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

Ok sweet was able to get it to build with your fork and changes!

There's just the final complaint (severity level 6) of having two input with the same id value of search-input.

Screenshot 2023-07-20 at 2 22 13 PM

@hkang1 hkang1 assigned ashtonG and unassigned hkang1 Jul 20, 2023
@ashtonG ashtonG requested a review from hkang1 July 21, 2023 16:16
@determined-ci
Copy link
Collaborator

Docsite preview being generated for this PR.
You can (eventually) find the generated docsite here.

Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the changes!

@hkang1 hkang1 removed their assignment Jul 21, 2023
@ashtonG ashtonG merged commit 9caaae8 into main Jul 24, 2023
19 of 20 checks passed
@ashtonG ashtonG deleted the chore/WEB--1292/docs-accessibility branch July 24, 2023 14:27
@dannysauer dannysauer added this to the 0.23.4 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants