Skip to content

fix(datetime): date and time picker accessibility#25980

Closed
sean-perkins wants to merge 4 commits into
mainfrom
FW-2184
Closed

fix(datetime): date and time picker accessibility#25980
sean-perkins wants to merge 4 commits into
mainfrom
FW-2184

Conversation

@sean-perkins
Copy link
Copy Markdown
Contributor

@sean-perkins sean-perkins commented Sep 20, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
    • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Picker Internal:

  • The input mode applies to all columns, regardless of which is active. e.g. When the time picker is opened, typing "24" will activate minute "24", even though the hour column is the first active column.

Datetime has several accessibility issues:

  • The icon in the month/year dropdown is announced to screen readers
  • Previous, current and next calendar months are announced to screen readers (e.g. non-visible months are read)
  • Users cannot navigate focus easily from the calendar body to the time picker
  • Time picker does not announce value changes to screen readers
  • Time picker does not announce total available options in each column
  • Time picker does not announce the type of column being focused

Issue URL: Internal

What is the new behavior?

Picker Internal

  • The input mode behavior only applies to the focused column. e.g.: When the time picker is opened, typing "24" will not activate minute 24. Since the hour column is the focused column, first "2" will be focused and then hour "4" will be focused, since the hour column does not have an item for "24".

Accessibility

  • The icon in the month/year dropdown is not announced to screen readers
  • Only the active month is announced to screen readers
  • Users can easily navigate from the calendar body to the time picker
  • Time picker announces value changes to screen readers
  • Time picker announces the total number of items available in each column
  • Time picker announces the type of column being focused (e.g. "5 o'clock" or "10 minutes")

Does this introduce a breaking change?

  • Yes
  • No

Other information

Date picker

Before After
main-datetime-accessibility.mp4
pr-datetime-accessibility-v2.mp4
Outdated - shows announcement of selected item on focus (but interferes with input mode)
pr-datetime-accessibility.mp4

@github-actions github-actions Bot added the package: core @ionic/core package label Sep 20, 2022
this.isTimePopoverOpen = true;

popoverRef.present(
popoverRef.presentFromTrigger(
Copy link
Copy Markdown
Contributor Author

@sean-perkins sean-perkins Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using presentFromTrigger allows us to configure if the direct descendent should be focused when the popover is presented. We want to focus the direct descendent so the hours picker column is announced immediately (this happens on iOS in the New Event interface).

* is actively visible to the user.
*/
this.scrollActiveItemIntoView();
this.focusActiveItem();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Focusing the active picker column item on value change will cause the screen reader to accurately announce the value change.

}

private onFocus = () => {
this.focusActiveItem();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally we need to focus the active item manually when the picker column receives focus, so that the active item is correctly announced.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to make this improvement right now. The input mode implementation is not compatible with the button being focused when the column is focused. The input requires the keypress events to be bubbled from the picker column being focused.

return (
<button
tabindex="-1"
role="radio"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Role radio seemed like the most appropriate role based on the ARIA spec (single selection allowed in a group at a time).

<div class="picker-item picker-item-empty">&nbsp;</div>
<div class="picker-item picker-item-empty">&nbsp;</div>
<div class="picker-item picker-item-empty">&nbsp;</div>
<div class="picker-item picker-item-empty" aria-hidden="true">
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty picker items are hidden from screen readers, otherwise they are read when the screen reader announces the entire section.

return (
<div
// Non-visible months should be hidden from screen readers
aria-hidden={!isWorkingMonth ? 'true' : 'false'}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding the months outside of the active working month, results in the screen reader not announcing the previous and next month days when the group is announced.

{getMonthAndYear(this.locale, this.workingParts)}{' '}
<ion-icon icon={this.showMonthAndYear ? expandedIcon : collapsedIcon} lazy={false}></ion-icon>
<ion-icon
aria-hidden="true"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This icon was being announced as "image" when the section was being read. Hiding it from screen readers as that information is not valuable or useful.

@sean-perkins sean-perkins marked this pull request as ready for review September 21, 2022 15:13
@sean-perkins sean-perkins requested a review from a team September 21, 2022 15:13
Copy link
Copy Markdown
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we break these changes out into separate PRs? FW-2184 only deals with the calendar body

@sean-perkins
Copy link
Copy Markdown
Contributor Author

Sure thing!

@sean-perkins
Copy link
Copy Markdown
Contributor Author

All behavior in this PR have been split into separate pull requests; closing.

@sean-perkins sean-perkins deleted the FW-2184 branch September 27, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants