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

Add audits in the Jupyter ecosystem #133

Merged
merged 5 commits into from
Jul 20, 2023
Merged

Conversation

steff456
Copy link
Contributor

@steff456 steff456 commented Jun 29, 2023

This PR,

  • Creates a documentation page with the zoom audit
  • Creates a documentation page with the keyboard navigation audit
  • Creates a documentation section for the audits
  • Links the new page for adding it to the side menu
  • Links the new section for it to be rendered in the side menu
  • Adds reference to other audits that happened in the last 2 years

This PR is part of #125


📚 Documentation preview 📚: https://jupyter-accessibility--133.org.readthedocs.build/en/133/

@tonyfast
Copy link
Contributor

is there anyway to include other audits that have been done? it seems diminishing of other's efforts to no include them. at the very least, i think it would be working mentioning:

@steff456
Copy link
Contributor Author

I'm going to add them in follow up PRs, please see #125

I want to stick with one audit per PR for it to be easier to review

@tonyfast
Copy link
Contributor

thanks for sharing such a thorough roadmap. those improvements look great!

@trallard trallard added area: documentation 📖 Accessible user documentation PR: needs review 👀 This PR is completed and someone needs to have a look type: audit labels Jun 29, 2023
@steff456 steff456 changed the title Add zoom audit Add audits in the Jupyter ecosystem Jul 6, 2023
@steff456
Copy link
Contributor Author

steff456 commented Jul 6, 2023

Edit: I pushed the changes for all the audits, so now this PR will add documentation for all the accessibility audits

Copy link
Collaborator

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

My opinion is that the audits page should mostly just link out to content elsewhere, rather than adding / duplicating content to the docs.

@@ -0,0 +1,24 @@
# Jupyter Accessibility Audits
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I were someone coming to this page for the first time, I might wonder why some of the audits are in GitHub issues, while two are in these docs. Is there some way we can address that? Or maybe put all the audit links in one list, rather than splitting them into two lists?

I'm not sure what the best approach is, but currently this looks confusing to me.

Copy link
Member

Choose a reason for hiding this comment

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

Agree - the current structure (two lists, some with in-depth content other without) make this confusing at the least, and signals some sort of "hierarchy" / "importance at worst"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I only add the links to the github issues/PR?

Still we need to add the new two pages to a toctree, if not Sphinx will not include them and will not be reachable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edit: I removed the single audit pages and just left the list of all the audits

docs/audits/index.md Outdated Show resolved Hide resolved
docs/audits/index.md Outdated Show resolved Hide resolved
docs/audits/keyboard-audit.md Outdated Show resolved Hide resolved
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

This PR needs some restructuring, as discussed separately the main content is in the issues themselves, these docs should point to those, avoid repeating content and if needed provide only essential context.

docs/audits/index.md Outdated Show resolved Hide resolved
docs/audits/index.md Outdated Show resolved Hide resolved
docs/audits/keyboard-audit.md Outdated Show resolved Hide resolved
docs/audits/zoom-audit.md Outdated Show resolved Hide resolved
docs/audits/zoom-audit.md Outdated Show resolved Hide resolved
docs/audits/zoom-audit.md Outdated Show resolved Hide resolved
docs/_toc.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,24 @@
# Jupyter Accessibility Audits
Copy link
Member

Choose a reason for hiding this comment

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

Agree - the current structure (two lists, some with in-depth content other without) make this confusing at the least, and signals some sort of "hierarchy" / "importance at worst"

docs/audits/index.md Outdated Show resolved Hide resolved
docs/audits/index.md Outdated Show resolved Hide resolved
@trallard trallard added PR: changes requested 🏗 This PR has been reviewed and needs changes and removed PR: needs review 👀 This PR is completed and someone needs to have a look labels Jul 12, 2023
Copy link
Contributor

@isabela-pf isabela-pf left a comment

Choose a reason for hiding this comment

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

I'm really happy to see these collected somewhere. Thank you for making this a priority! And thank you for the descriptive link text. I love that the dates are made clear.

This looks good to me, and it previews well. I'd like to merge it, but GitHub is telling me there are changes still requested. It looks like the changes have been made but not rereviewed after, so I'll check back and see if I can merge.

@isabela-pf isabela-pf merged commit 3420956 into jupyter:main Jul 20, 2023
1 of 2 checks passed
@steff456 steff456 deleted the add-zoom-audit branch July 20, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation 📖 Accessible user documentation PR: changes requested 🏗 This PR has been reviewed and needs changes type: audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants