[terra-date-picker] Fix accessibility issues. #1252
Conversation
pranav300
requested review from
supreethmr,
lokesh-0813,
saket2403,
ShettyAkarsh and
avinashg1994
pranav300
requested review from
benbcai,
dkasper-was-taken,
mjhenkes,
ryanthemanuel and
tbiethman
as code owners
| if (help && error && isInvalid) { | ||
| if (error && isInvalid) { |
There was a problem hiding this comment.
I think the help condition is needed if we are applying ${datePickerId}-help to ariaDescriptionIds.
There was a problem hiding this comment.
We could probably break this out to two single checks like:
if (help) {
ariaDescriptionIds = `${ariaDescriptionIds} ${datePickerId}-help`;
}
if (error && isInvalid) {
ariaDescriptionIds = `${ariaDescriptionIds} ${datePickerId}-error`;
}not sure if that is simpler or not
There was a problem hiding this comment.
I think it should explicitly check for help if we're adding ${datePickerId}-help.
There was a problem hiding this comment.
What my idea was to always provide help for DatePickerField because even if the user has not provided any help message but the format message/help message will always be coming from DatePickerField.
There was a problem hiding this comment.
That makes sense. Adding a comment here may help clarify the confusion.
| } else if (inputAttributes && inputAttributes['aria-describedby']) { | ||
| this.formatDescriptionId = inputAttributes['aria-describedby']; | ||
| } else { | ||
| this.formatDescriptionId = ''; |
There was a problem hiding this comment.
Given the change made in DatePickerField where aria-describedby will always be provided, it should never get in here. If it ever gets in here, should this be set to undefined instead of an empty string so that aria-describedby is not defined instead of setting to an empty string?
There was a problem hiding this comment.
Updated here fbcd7ea
jeremyfuksa
added
⭐ Accessibility Reviewed
⭐ UX Reviewed
| @@ -219,8 +229,9 @@ class DatePickerInput extends React.Component { | |||
| onChange={this.handleOnChange} | |||
| onFocus={onFocus} | |||
| onBlur={onBlur} | |||
| aria-required={required} | |||
There was a problem hiding this comment.
Added this so that voiceover and jaws read required after the label when the field component is required.
# Conflicts: # packages/terra-date-picker/tests/jest/__snapshots__/DateInput.test.jsx.snap # packages/terra-date-picker/tests/jest/__snapshots__/DatePicker.test.jsx.snap
Summary
This PR removes the duplicate id
formatfromDatePickerField. And cleans up the aria description id's for better accessibility.Part of #1193
Similar to #1244
Deployment Link
https://terra-framework-deployed-pr-#.herokuapp.com/
Testing
Added a new test with multiple Datepickerfield.
Added wdio test for the same
Additional Details
Current behavior:
Updated behavior:
Thank you for contributing to Terra.
@cerner/terra