FIX: Replace the AM/PM toggle ButtonGroup with ToggleGroupControl. #61562

patil-vipul · 2024-05-10T07:21:37Z

What?

Why?

When the is12Hour={true} setting is enabled, the AM/PM toggle relies solely on visual cues to convey its state, which presents a challenge for screen readers. This design overlooks accessibility needs, potentially creating barriers for users who rely on such assistive technologies.

How?

The previous ButtonGroup for toggling between AM/PM is replaced with ToggleGroupControl for better accessibility.

Testing Instructions

Create a new post and add the Date block.
Make sure your site is configured for 12 Hrs Time Format from the Settings >> General
Now edit the time on the date block using the "Edit" option (pencil icon) in the Block Controls Toolbar.
Test using the screen reader to read aloud the intended use of ToggleGroupControl.

Testing Instructions for Keyboard

Create a new post and add the Date block.
Make sure your site is configured for 12 Hrs Time Format from the Settings >> General
Now edit the time on the date block using the "Edit" option (pencil icon) in the Block Controls Toolbar.
Navigate to AM/PM selector using Tab key.
Once on AM/PM selector you can toggle between the two states using arrow keys.
To select the highlighted option press Spacebar.

Screenshots or screencast

The previous ButtonGroup for toggling between AM/PM is replaced with ToggleGroupControl for better accessibility.

github-actions · 2024-05-10T07:21:54Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: patil-vipul <vipulpatil@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2024-05-10T07:22:08Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @patil-vipul! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

mirka

Great work here, thank you for the contribution!

mirka · 2024-05-22T12:53:06Z

packages/components/src/date-time/time/index.tsx

-									am === 'AM' ? 'primary' : 'secondary'
-								}
-								__next40pxDefaultSize
+								aria-label="AM"
 								onClick={ buildAmPmChangeCallback( 'AM' ) }


Instead of handling changes on the click handlers for each option, we want it to be handled in the onChange of the ToggleGroupControl component itself. You can observe the difference when you try to change the value using keyboard arrow keys, rather than clicking.

mirka · 2024-05-22T12:54:11Z

packages/components/src/date-time/time/index.tsx

-									am === 'AM' ? 'primary' : 'secondary'
-								}
-								__next40pxDefaultSize
+								aria-label="AM"


Suggested change

aria-label="AM"

If the aria-label is the same as the label, we only need the label.

mirka · 2024-05-22T15:31:54Z

packages/components/src/date-time/time/test/index.tsx

@@ -165,7 +165,7 @@ describe( 'TimePicker', () => {
 			/>
 		);

-		const pmButton = screen.getByText( 'PM' );
+		const pmButton = screen.getByLabelText( 'PM', { selector: 'button' } );


Suggested change

const pmButton = screen.getByLabelText( 'PM', { selector: 'button' } );

const pmButton = screen.getByRole( 'radio', { name: 'PM' } );

This would be the query we want to assert the correct accessibility semantics (same with the other instances).

mirka · 2024-05-22T15:32:37Z

packages/components/src/date-time/time/test/index.tsx

 		 * This is not ideal, but best of we can do for now until we refactor
 		 * AM/PM into accessible elements, like radio buttons.
 		 */


I believe we can remove this comment now?

mirka · 2024-05-22T15:39:16Z

packages/components/CHANGELOG.md

@@ -2,6 +2,10 @@

 ## Unreleased

+### Internal
+
+-   Replaced `ButtonGroup` with `ToggleGroupControl` component for "AM/PM" selector in DateTime component ([#61562](https://github.com/WordPress/gutenberg/pull/61562)).


Suggested change

- Replaced `ButtonGroup` with `ToggleGroupControl` component for "AM/PM" selector in DateTime component ([#61562](https://github.com/WordPress/gutenberg/pull/61562)).

- `DateTimePicker`: Replaced `ButtonGroup` with `ToggleGroupControl` component for "AM/PM" selector ([#61562](https://github.com/WordPress/gutenberg/pull/61562)).

Just a small tweak for easier scanning. Also, I think we can classify this as an Enhancement? Or maybe even a Bug Fix, since this fixes an accessibility bug. No strong opinion though.

Replace the AM/PM toggle ButtonGroup with ToggleGroupControl.

9943802

The previous ButtonGroup for toggling between AM/PM is replaced with ToggleGroupControl for better accessibility.

patil-vipul requested a review from ajitbohra as a code owner May 10, 2024 07:21

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label May 10, 2024

juanfra added [Type] Enhancement A suggestion for improvement. [Block] Query Loop Affects the Query Loop Block [Block] Post Template Affects the Post Template Block labels May 10, 2024

patil-vipul added 8 commits May 10, 2024 16:22

Update the AM/PM toggle tests with respect to ToggleGroupControl.

db9f595

Merge branch 'trunk' into fix/datetime-picker-accessibility

e06249e

Update the AM/PM toggle tests with respect to ToggleGroupControl.

ae92dde

Merge branch 'trunk' into fix/datetime-picker-accessibility

8b6cfac

Update the AM/PM toggle tests with respect to ToggleGroupControl.

353e851

Updated CHANGELOG.md

d367263

Merge branch 'trunk' into fix/datetime-picker-accessibility

f665377

Moved text within CHANGELOG.md to appropriate position.

a740dcc

patil-vipul mentioned this pull request May 17, 2024

DateTimePicker: AM/PM switcher is not screen reader accessible #61163

Open

patil-vipul and others added 3 commits May 17, 2024 12:22

FIX: Add missing value parameter to ToggleGroupControl.

da43be9

Merge branch 'trunk' into fix/datetime-picker-accessibility

5698061

Merge branch 'trunk' into fix/datetime-picker-accessibility

913f31e

mirka requested a review from a team May 20, 2024 15:45

mirka added the [Package] Components /packages/components label May 20, 2024

mirka reviewed May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: Replace the AM/PM toggle ButtonGroup with ToggleGroupControl. #61562

FIX: Replace the AM/PM toggle ButtonGroup with ToggleGroupControl. #61562

patil-vipul commented May 10, 2024 •

edited

github-actions bot commented May 10, 2024 •

edited

github-actions bot commented May 10, 2024

mirka left a comment

mirka May 22, 2024

mirka May 22, 2024

mirka May 22, 2024

mirka May 22, 2024

mirka May 22, 2024

	const pmButton = screen.getByLabelText( 'PM', { selector: 'button' } );
	const pmButton = screen.getByRole( 'radio', { name: 'PM' } );

	- Replaced `ButtonGroup` with `ToggleGroupControl` component for "AM/PM" selector in DateTime component ([#61562](https://github.com/WordPress/gutenberg/pull/61562)).
	- `DateTimePicker`: Replaced `ButtonGroup` with `ToggleGroupControl` component for "AM/PM" selector ([#61562](https://github.com/WordPress/gutenberg/pull/61562)).

FIX: Replace the AM/PM toggle ButtonGroup with ToggleGroupControl. #61562

Are you sure you want to change the base?

FIX: Replace the AM/PM toggle ButtonGroup with ToggleGroupControl. #61562

Conversation

patil-vipul commented May 10, 2024 • edited

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented May 10, 2024 • edited

github-actions bot commented May 10, 2024

mirka left a comment

Choose a reason for hiding this comment

mirka May 22, 2024

Choose a reason for hiding this comment

mirka May 22, 2024

Choose a reason for hiding this comment

mirka May 22, 2024

Choose a reason for hiding this comment

mirka May 22, 2024

Choose a reason for hiding this comment

mirka May 22, 2024

Choose a reason for hiding this comment

patil-vipul commented May 10, 2024 •

edited

github-actions bot commented May 10, 2024 •

edited