Improve/Fix Accessibility for WeekdayPicker Component for Screen reader #6055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ragesoss
reviewed
Dec 10, 2024
| @@ -151,6 +152,7 @@ const Calendar = createReactClass({ | |||
|
|
|||
| const onWeekdayClick = this.props.editable ? this.selectWeekday : null; | |||
| const onDayClick = this.props.editable ? this.selectDay : null; | |||
| const ariaModifier = this.props.useAriaLabel ? 'selected' : null; | |||
There was a problem hiding this comment.
Why is it necessary to make changes outside of the WeekdayPicker component for this?
There was a problem hiding this comment.
Actually, I reviewed the code again and realized this change is not necessary. I have removed the existing ariaModifier since the WeekdayPickerComponent was not using it at all, and the Calendar component was not passing it either. I replaced it with the hardcoded string selected.
Additionally, I removed aria-pressed and replaced it with aria-label & aria-live.
Also, since the WeekdayPickerComponent is only used by the Calendar component, this change should not cause any issues.
Can you please review the updated code whenever you have time? Thank you 😃
Abishekcs
force-pushed
the
fix/screen-reader-weekdaypicker-accessibility
branch
2 times, most recently
from
ca0e166 to
82281b9
Compare
…ve support - Add `aria-label` and `aria-live` for improved screen reader compatibility. - Add CSS styling for the `aria-live` region to ensure proper visibility and user experience. - remove aria-pressed
Abishekcs
force-pushed
the
fix/screen-reader-weekdaypicker-accessibility
branch
from
82281b9 to
a7eaabc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #6051
What this PR does
This PR fixes the accessibility of the Calendar and WeekdayPicker components by introducing the
aria-labeland removingaria-pressed. And propariaModifierwhich was not being used was replaced by a simple hardcoded string which is used to detect if a "weekday" is selected.Cause of the Bug:
The
ariaModifierprop (of typePropTypes.string) required by the WeekdayPicker component was not being passed from the Calendar component or any of its parent components. This prop is essential for determining whether a button has been pressed and updating thearia-pressedattribute accordingly. As a result, thearia-pressedattribute remainedfalseeven when the button was pressed because it depended on the correct value ofariaModifier.Which was not passed in the first place itself.Screenshots
Before:
Before.mp4
After:
2024-12-11.15-43-50.mp4
Open questions and concerns
I applied CSS to hide the
aria-livediv (weekday_picker.jsx), using a style similar to that already used for hiding admin notes. Although the two elements use different class names, the CSS properties are the same. I considered using a single shared CSS class for both cases but was uncertain about the best file to place this shared CSS in.