Skip to content
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

fix(material/datepicker): calendar aria-descriptions start/end date #25457

Merged
merged 1 commit into from Aug 26, 2022

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Aug 12, 2022

For date ranges, add aria-descriptions to the cell of the current start
date and also for end date.

Popuplate aria descriptions with the expected value of the ARIA accessible name of the
matStartDate and matEndDate inputs. Introduces _computeAriaAccessibleName function to
implement ARIA acc-name-1.2 specificiation.

Fixes #23442 and #23445

Screen Shot 2022-08-19 at 2 20 14 PM

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/datepicker labels Aug 12, 2022
@zarend
Copy link
Contributor Author

zarend commented Aug 12, 2022

This is pretty far along, but still a draft. Here's the remaining TODO's to make this ready:

  • Fix examples to have both placeholder and aria-label
  • rename startdate-arialabelledby to startdate-accessible-name
  • update documentation to reflect best practice of giving matStartDate/matEndDate an aria label. (I plan on sending out a separate PR for documentation).
  • address 3 failing CI checks

@zarend zarend changed the title fix(material/datepicker): add aria-descriptions to calendar for start… fix(material/datepicker): calendar aria-descriptions start/end date Aug 18, 2022
@zarend zarend added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Aug 18, 2022
@zarend zarend marked this pull request as ready for review August 18, 2022 00:10
src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
src/material/datepicker/calendar-body.html Outdated Show resolved Hide resolved
src/material/datepicker/calendar-body.ts Outdated Show resolved Hide resolved
src/material/datepicker/calendar.ts Show resolved Hide resolved
src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
@zarend zarend force-pushed the date-range-aria branch 2 times, most recently from 314b631 to b943338 Compare August 19, 2022 21:21
@zarend zarend requested a review from crisbeto August 19, 2022 21:22
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

src/material/datepicker/aria-accessible-name.ts Outdated Show resolved Hide resolved
@zarend
Copy link
Contributor Author

zarend commented Aug 21, 2022

@crisbeto my thought process here is that I recommend using _computeAriaAccessibleName only as a last resort. In most situations I would expect there to be an easier solution than predicting what the ARIA api will do 😂 . I typed the argument as HTMLInputElement because the unit tests only cover the Datepicker's use case.

tomorrow, I can take a look at HTMLTextareaElement and see what it would take to add that. IIRC text area would be almost identical to input.

@crisbeto
Copy link
Member

I was mostly referring to setting the accepted types to HTMLInputElement|HTMLTextareaElement, I don't think that we need to unit test textarea specifically.

@zarend
Copy link
Contributor Author

zarend commented Aug 22, 2022

Okay, we can do that, we would also need to change this line that detects input element to: if (ssrSafeIsHTMLInputElement(currentNode) || ssrSafeIsTextareaElement(currentNode)) {. That way we properly handle the placeholder attribute, title attribute, and for/id with <textarea/>.

Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

For date ranges, add aria-descriptions to the cell of the current start
date and also for end date.

Popuplate aria descriptions with the expected value of the ARIA accessible name of the
`matStartDate` and `matEndDate` inputs. Introduces `_computeAriaAccessibleName` function to
implement ARIA acc-name-1.2 specificiation.

Fixes angular#23442 and angular#23445
@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Aug 26, 2022
@zarend zarend merged commit 67212ba into angular:main Aug 26, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Sep 16, 2022
…ngular#25457)

For date ranges, add aria-descriptions to the cell of the current start
date and also for end date.

Popuplate aria descriptions with the expected value of the ARIA accessible name of the
`matStartDate` and `matEndDate` inputs. Introduces `_computeAriaAccessibleName` function to
implement ARIA acc-name-1.2 specificiation.

Fixes angular#23442 and angular#23445
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker area: material/datepicker target: minor This PR is targeted for the next minor release
Projects
None yet
3 participants