[date-time-picker] Defect fixes #2036
Conversation
ashishkumbhare116
changed the title
| /> | ||
| </Field> | ||
| <Fieldset | ||
| type="checkbox" |
There was a problem hiding this comment.
why is type attribute used here?
There was a problem hiding this comment.
Not needed .Changed 08f942a
| @@ -801,8 +801,9 @@ class DatePicker extends React.Component { | |||
| isCalendarOpenedViaKeyboard={this.state.isCalendarOpenedViaKeyboard} | |||
| initialTimeZone={this.props.initialTimeZone} | |||
| > | |||
| <VisuallyHiddenText aria-atomic="true" aria-live="assertive" refCallback={(ref) => { this.visuallyHiddenText = ref; }} className={cx('react-datepicker-visuallyhiddentext')}> | |||
There was a problem hiding this comment.
This change is not valid since it prevents SR from announcing the dates when navigated with arrow keys.
There was a problem hiding this comment.
Yes Right Have 50a6cc2 implemented changes.
| const isValidDateSelection = date ? isDayInRange(date, this.props.minDate, this.props.maxDate) : true | ||
| if (isValidDateSelection) { | ||
| this.setState({ | ||
| preSelection: date | ||
| }) | ||
| type === "month" ? this.updateAriaLiveStatus(getMonthFromDate(date, this.props)) : |
There was a problem hiding this comment.
is there any existing getmonth utility that can be used ?
what is this returning? https://github.com/cerner/terra-framework/blob/main/packages/terra-date-picker/src/react-datepicker/date_utils.js#L121
There was a problem hiding this comment.
No this one is returning the numeric value for month .
There was a problem hiding this comment.
For the month string value required to create new utility function
| const isValidDateSelection = date ? isDayInRange(date, this.props.minDate, this.props.maxDate) : true | ||
| if (isValidDateSelection) { | ||
| this.setState({ | ||
| preSelection: date | ||
| }) | ||
| type === "month" ? this.updateAriaLiveStatus(getMonthFromDate(date, this.props)) : | ||
| type === "year" ? this.updateAriaLiveStatus(value, this.props) : |
There was a problem hiding this comment.
use constants for month and year here and other places where method is called (if doesnt exist)
There was a problem hiding this comment.
46534ab Added
| const isValidDateSelection = date ? isDayInRange(date, this.props.minDate, this.props.maxDate) : true | ||
| if (isValidDateSelection) { | ||
| this.setState({ | ||
| preSelection: date | ||
| }) | ||
| type === "month" ? this.updateAriaLiveStatus(getMonthFromDate(date, this.props)) : | ||
| type === "year" ? this.updateAriaLiveStatus(value, this.props) : |
There was a problem hiding this comment.
updateAriaLiveStatus accepts only one arg.
There was a problem hiding this comment.
This one added 5 years back and followed the same flow, second prop is internally using the date_util's intl prop value without using it is throwing an error as “cannot read property of intl "
| const isValidDateSelection = date ? isDayInRange(date, this.props.minDate, this.props.maxDate) : true | ||
| if (isValidDateSelection) { | ||
| this.setState({ | ||
| preSelection: date | ||
| }) | ||
| type === "month" ? this.updateAriaLiveStatus(getMonthFromDate(date, this.props)) : | ||
| type === "year" ? this.updateAriaLiveStatus(value, this.props) : | ||
| this.updateAriaLiveStatus(getLocalizedDateForScreenReader(date, this.props)); |
There was a problem hiding this comment.
how is this not overwriting the previous lines (even though SR announces only month/year when changed). Can this line be re-checked once
There was a problem hiding this comment.
No it is not overwriting the previous lines have cross checked as this is similar to this in if & else condition if (type === 'month') {
this.updateAriaLiveStatus(getMonthFromDate(date, this.props));
} else if (type === 'year') {
this.updateAriaLiveStatus(value, this.props);
} else {
this.updateAriaLiveStatus(getLocalizedDateForScreenReader(date, this.props));
}
| const isValidDateSelection = date ? isDayInRange(date, this.props.minDate, this.props.maxDate) : true | ||
| if (isValidDateSelection) { | ||
| this.setState({ | ||
| preSelection: date | ||
| }) | ||
| type === "month" ? this.updateAriaLiveStatus(getMonthFromDate(date, this.props)) : | ||
| type === "year" ? this.updateAriaLiveStatus(value, this.props) : |
There was a problem hiding this comment.
| type === "year" ? this.updateAriaLiveStatus(value, this.props) : | |
| type === "year" ? this.updateAriaLiveStatus(value) : |
There was a problem hiding this comment.
Yes ,sorry missed it 81d4628
|
+1 for the latest a11y changes, |
Summary
Invalid field SR announcement fix & Calendar von value change SR announcement fix .
Doc/Example update for TimeZone SR announcement.
What was changed:
Why it was changed:
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-10236
UXPLATFORM-10237
Thank you for contributing to Terra.
@cerner/terra