-
Notifications
You must be signed in to change notification settings - Fork 8
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
Adding eslint-plugin-jsx-a11y plugin and general accessibility #1143
base: nextjs
Are you sure you want to change the base?
Conversation
…ble through aria-labels/key press functionality in conjunction with existing on click methods
Quality Gate failedFailed conditions |
@@ -37,7 +37,7 @@ const LearnMod = () => { | |||
<div className="classes"> | |||
{content.map((lesson, index) => { | |||
//const moduleProgress = user.userProgressData[lesson.moduleID]; | |||
|
|||
console.log(lesson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this debug print needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover from testing, will clean up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a quick comment. Have one of the project leads review
Thanks for this PR @bagnishant ! Glad to see this :) I wonder if there is any sort of wiki you used to identify best practices for write a11y labels for some of the UI components. @farisdurrani any thoughts? |
Food for thought: Are there any tools to see if we can ensure that any key components/buttons that the user will interact with are as accessible as possible? ie: could we have some automated check to say that for certain pages, we should cross some certain threshold score in "how accessible" is this page for say those with screen readers? @farisdurrani @bagnishant what are your thoughts? |
@karkir0003 I currently have the eslint-plugin-jsx-a11y plugin set to the "recommended" configuration, but you can change it to the "strict" configuration to have the plugin check the code more meticulously for accessibility options/details. Also, we can also have a custom configuration to choose specific rules the plugin supports to enforce. |
@bagnishant build checks failing. plz fix |
Adding eslint-plugin-jsx-a11y plugin and general accessibility
Github Issue Number: #932
What problem are we solving?
This PR aims to solve potential accessibility issues that certain users may face, preventing them from properly understanding the content on the website.
What solution does this PR provide?
This PR adds the eslint-plugin-jsx-a11y plugin, which aids in detecting potential accessibility issues in the static code and led me to make appropriate changes (for example, adding onKeyDown attributes to certain buttons and tabs to match onClick behavior). In addition, I looked through the DOM over several common pages/components that users may often used and added aria-labels to elements without text explaining the purpose of the element.
Testing Methodology
For changes such as adding onKeyPress functionality, I made sure to see what the onClick function did beforehand and reverified its functionality after my changes. For aria-labels, I looked to see how the element was presented on the page and how it looked like in the DOM; after adding labels, I made sure that its display on the screen was the same, but an aria-label was displayed on the DOM. Most of my changes were simply adding accessibility-related attributes to existing elements.
Video showcasing changes
https://youtu.be/rVwvaCLJ7-I