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/timepicker): adds default aria-label to timepicker toggle #30284

Merged

Conversation

essjay05
Copy link
Contributor

@essjay05 essjay05 commented Jan 7, 2025

Updates Angular Component Timepicker so the timepicker toggle has a default aria-label value if no aria-label is applied for the timepicker toggle button to make it more accessible. Also adds additional option of applying aria-labelledby to Timepicker toggle button.

Before screencast
After screencast

Fixes b/380308482

@essjay05 essjay05 requested a review from a team as a code owner January 7, 2025 22:56
@essjay05 essjay05 requested review from amysorto and andrewseguin and removed request for a team January 7, 2025 22:56
@wagnermaciel wagnermaciel added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Jan 7, 2025
Copy link

github-actions bot commented Jan 7, 2025

Deployed dev-app for fc37006 to: https://ng-dev-previews-comp--pr-angular-components-30284-dev-gnu2ezzl.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from 2252c9a to c91c98a Compare January 8, 2025 21:16
@essjay05 essjay05 changed the title fix(material/timepicker): adds aria-labelledby to timepicker toggle fix(material/timepicker): adds default aria-label to timepicker toggle Jan 8, 2025
@essjay05 essjay05 marked this pull request as draft January 8, 2025 23:27
@essjay05 essjay05 marked this pull request as ready for review January 8, 2025 23:48
@essjay05 essjay05 requested a review from crisbeto January 8, 2025 23:48
@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from 1174e6d to 91fbc3e Compare January 8, 2025 23:49
@wagnermaciel
Copy link
Contributor

You'll need to update the api goldens. I believe this is the command:

yarn bazel run //tools/public_api_guard:material/timepicker.md_api.accept

@angular-robot angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label Jan 9, 2025
@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from 658d59c to fd5f7ab Compare January 9, 2025 17:45
@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from 2ceb536 to 1064b5f Compare January 10, 2025 18:13
@essjay05 essjay05 requested a review from crisbeto January 10, 2025 18:48
@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from 1064b5f to aa98fb6 Compare January 14, 2025 18:26
Copy link
Contributor

@wagnermaciel wagnermaciel left a comment

Choose a reason for hiding this comment

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

LGTM

Updates Angular Component Timepicker so the timepicker toggle uses
the aria-labelledby value for the timepicker toggle button to make
it more accessible.

Fixes b/380308482
Updates Angular Components Timepicker component by creating a
default aria-label to the timepicker toggle if one is not supplied.
This makes the button more accessible and descriptive for screen
readers.

Fix b/380308482
Updates previous fix to fix lint error.
Ran command to update api goldens.
…rride

Updates previous fix which implements a default aria-label if no custom
aria-label is provided to where it checks if an aria-lablledby value is
provided and uses the aria-labelledby value if available. If not it will
check for a custom aria-label and if that is not provided it will default
to the generic aria-label value.
@essjay05 essjay05 force-pushed the add-timepicker-toggle-aria-labelledby branch from aa98fb6 to fc37006 Compare January 15, 2025 01:12
@andrewseguin andrewseguin added the action: merge The PR is ready for merge by the caretaker label Jan 15, 2025
@andrewseguin andrewseguin added the target: patch This PR is targeted for the next patch release label Jan 15, 2025
@andrewseguin andrewseguin merged commit e5c3ddf into angular:main Jan 15, 2025
20 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project area: material/timepicker dev-app preview When applied, previews of the dev-app are deployed to Firebase target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants