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

Re-add jsx-a11y and accessibility fixes #416

Merged
merged 7 commits into from
Feb 29, 2024
Merged

Re-add jsx-a11y and accessibility fixes #416

merged 7 commits into from
Feb 29, 2024

Conversation

js0mmer
Copy link
Member

@js0mmer js0mmer commented Feb 1, 2024

Description

This PR adds back jsx-a11y (previously included with react-scripts) and fixes relevant accessibility errors
Accessiblity fixes:

  • Made search module hit items accessible
    • they use divs so I assigned a "button" role and a tab index so they can be focused on when tabbing through the page
    • added a listener for enter key pressing down in addition to the mouse down listener
  • Replaced improper usage of label element in reports page
    • label is for labeling inputs only
  • Added alt tags where needed
  • Replaced theme toggle anchor tag in sidebar with button
  • Refactored how dragging a course from search into the planner works on the roadmap page
    • desktop version previously relied on mouse down listener to set the course that was actively being dragged
      • this isn't accessible - doesn't work if a user isn't using a mouse
    • made use of the drag start listener and the search slice to set the active course that is being dragged
    • mobile version has enter key listener now
  • made profile image a button
  • made overlaytriggers on roadmap accessible by using a button and using functional form (which always gets rid of strict mode warnings)

Screenshots

tab.accessibility.mp4

Steps to verify/test this change:

  • Verify changes work as expected on staging instance
    • test tab accessibility - use spacebar to start/end drag for drag/drop components and arrow keys to move
    • test dragging from search on roadmap

Final Checks:

  • Verify successful deployment

(optional)

  • Write tests
  • Write documentation

Issues

Closes #415

@js0mmer js0mmer self-assigned this Feb 1, 2024
Copy link

github-actions bot commented Feb 7, 2024

Deployed staging instance to https://staging-416.peterportal.org

Copy link
Contributor

@SenghoungLim SenghoungLim 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 overall it is great, especially the catalog page. A few things about the roadmap page, (1) I think when a user tab to the drop down menu on the roadmap year, we should have a rectangle to indicate that the tab is there as well for consistent and visual.
Gif showing the year drop down tab that has no rectangle indicate selection on it

(2) I think the same with the select verified and add review:
Gif showing the select verified review and add review do not have rectangle select visual

(3) Lastly, minor thing probably from previous code, we should remove console.log(review):
console log image

@js0mmer
Copy link
Member Author

js0mmer commented Feb 29, 2024

Will merge this as is! Issues with accordion outlines and popover accessibility can be dealt with in a later PR (or two) as discussed in today's meeting since they don't fall under the scope of jsx-a11y lint errors.

@js0mmer js0mmer merged commit eb4f3ee into master Feb 29, 2024
3 checks passed
@js0mmer js0mmer deleted the jsx-a11y branch February 29, 2024 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-add jsx-a11y
2 participants