Fix time picker period selector a11y touch targets #171437

bleroux · 2025-07-01T13:08:24Z

Description

This PR fixes the TimePicker day period selector touch targets and semantics bounds.

	Before	After
dial mode portrait
dial mode landscape
input mode

Implementation choice

This PR implements two main changes:

it expands the _DayPeriodControl bounds by reducing some existing spacing. Doing so the touch area of the AM/PM buttons can grow outside the day period container and respect the minimum interactive height.
it expands the bounds of the AM/PM buttons semantics. In order to do it introduces a new public getter semanticsBoundsClippedByParent to RenderObject. This getter returns true by default and can be overriden to return false which allows a RenderObject.semanticsBounds to expand outside of parents clip bounds.

Side not, If semanticsBoundsClippedByParent is accepted, I will file a separate PR for it, with proper testing.

Related Issue

Fixes touch target size not up to a11y standards for DatePicker day period selector.

Tests

Adds 2 tests.

bleroux · 2025-07-01T14:01:22Z

@chunhtai This is an alternative PR to #170060.

I prefer this solution has it does not involve hacky solution for splitting the shape. Instead it adds a new API to allow semantics bounds to escape parent clipping.

Some context to why this is needed:

M3 tokens provide the shape of the container surronding both AM and PM button.
This translate in Flutter to a Material widget with Material.shape propertie sets according to the token.
As the two buttons have this Material widget as a parent their semantics bounds are clipped even when we try to return bounds which are a11y compliant.

chunhtai · 2025-07-02T17:42:14Z

packages/flutter/lib/src/rendering/object.dart

+  /// Wether the semantic bounding box will be clipped by [parent].
+  ///
+  /// Defaults to true,
+  bool get semanticsBoundsClippedByParent => true;


This only fixes for semantics rect. the tap target not only applies when screen reader is on, but also when people interact with the app directly. which means the gesture recognizer (and thus the renderobject) also needs to be above the tap target size

In this particular case (day period selector of the time picker), the size of the tap target is managed separately by the _DayPeriodInputPadding widget which takes care of the hit area (this class existed before this PR).

Recording showing that the hit target is also fixed by this PR (_DayPeriodInputPadding was there before but its size was wrong):

Screen.Recording.2025-07-03.at.16.11.12.mov

flutter-dashboard · 2025-07-03T14:31:16Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #171437 at sha 69eb1e4

flutter-dashboard · 2025-07-08T10:49:41Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #171437 at sha 4ef9a54

packages/flutter/lib/src/rendering/object.dart

Piinks · 2025-07-17T22:44:30Z

packages/flutter/lib/src/rendering/object.dart

-    Rect rect =
-        semanticsClipRect?.intersect(child.renderObject.semanticBounds) ??
-        child.renderObject.semanticBounds;
+    final bool shouldClip = child.renderObject.semanticsBoundsClippedByParent;


I am curious why we need changes to renderObject when in the Chip case, we were able to add semantics bounds exceeding the visual bounds of the Chip delete button?

The difference here is that the AM/PM buttons have a Material parent which clips their bounds and this apply to semantics bounds.
This new property is meant to opt-out from the default behavior (which is to clip semantics bounds).

It's probably uncommon to need this but the way the day period selector is defined in the M3 spec (a single shape that visually encloses both buttons) is a case where it would be helpful.

a single shape that visually encloses both buttons

Would it be easier if the buttons are visually enclosed in one shape, but under the hood they are separate? Chip too has a Material ancestor built-in, so I am not sure I follow yet why that causes this to require a different solution.

Thinking about the comment Chun-Heng made above about this only applying to the semantic rect and not the gesture detector, _DayPeriodInputPadding does indeed handle that here, but if this is helpful to be used in other scenarios, will the gesture need special handling every time?

Chip too has a Material ancestor built-in, so I am not sure I follow yet why that causes this to require a different solution.

Sorry, my description was not precise enough. It’s not the Material itself that leads to clipping, it’s the fact that Material.clipBehavior is set. In the Chip case it is not set by default. In the context of time picker it is set to Clip.antiAlias.
I pushed one commit which removes this clipBehavior. I also removed the new API I previously added to escape clipping of semantics bounds. With this change the buttons’ semantic bounds are ok but it leads to golden failures related to anti-aliasing not being applied.
Edit: and also the background color is overflowing on top corners when clip is not applied:

I still prefer the solution before this last commit, it adds a new API but it seems less fragile and it could be useful in other scenario, for instance, in the Chip case it would be useful because users can set InputChip.clipBehavior to something else than Clip.none.

Would it be easier if the buttons are visually enclosed in one shape, but under the hood they are separate?

Do you mean a solution similar to the one I tried in #170060?
In this other PR, the two buttons are separated under the hood: they both are responsible for drawing their own border. Which means each button has a SemanticNode at the top and a Material child which paints the button shape. Doing so the solution does not require a specific renderer and it is cleaner for the semantics part. Unfortunately, in order to achieve this result, because what we get from the spec is one single shape (and corresponding tokens), I had to rely on some logic to split this single shape into two shapes. To me it looks hacky and fragile (for instance, this can't work with any shape, drawing the separator is also problematic).

_DayPeriodInputPadding does indeed handle that here, but if this is helpful to be used in other scenarios, will the gesture need special handling every time?

Similar private widgets (named _InputPadding) are also defined in button_style_button.dart, in button.dart and in toggle_buttons.dart.
This looks like a recurring need. As you pointed, there are some variations related to gesture, after a quick look it seems that the part that differs is the implementation of hitTest. This is because the core feature of these widgets is to redirect tap events to a smaller inner child.
Do you think it would be valuable if I try to create a WIP PR which replace those various private widget with a single reusable one? Then we can discuss further if it is something we want to add as a public API.

bleroux · 2025-07-23T11:48:55Z

@Piinks I updated the PR with two commits:

one to fix the typo you pointed out
one to fix the golden failures that were previously reported (because this PR extends the day period selector bounds to respects a11y touch target, some paddings should be adjusted so that other parts of the UI does not move. Before this last commit, one padding was not adjusted resulting in the dialer moving south which led to golden failures).

flutter-dashboard · 2025-07-25T09:39:41Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #171437 at sha 3818659

chunhtai · 2025-08-01T19:30:58Z

packages/flutter/lib/src/material/time_picker.dart

+          minSize: Size(
+            dayPeriodSize.width,
+            math.max(dayPeriodSize.height, kMinInteractiveDimension),
+          ),
          orientation: orientation,
          child: SizedBox(
            height: dayPeriodSize.height,


should this be the same as minsize? also if this is the case we can keep the clip i think?

The SizedBox is used to set the visual height (dayPeriodSize.height is the visual height).
Here minSize is use to make the height accessible (kMinInteractive).

For instance, in landscape mode, the visual height of the period selector is 38, the a11y height is 48.
(see https://m3.material.io/components/time-pickers/specs#20e4c5c9-9e88-432d-99fc-02f6376b188c).

In portrait mode, visual height is 80, a11y height is 96 (48 for each button).

can you use marging to increase bounding size without increase the visual height? We should avoid adding render object API to get around the issue that parent clips child's semantics bound. we should find way to increase the parent's clip rect so that it won't clip the child

can you use marging to increase bounding size without increase the visual height?

The tree is _DayPeriodInputPadding (enforce touch height)-> Material (border and clip, enforce visual height) -> Column/Row -> [Semantics > AM InkWell, Semantics > PM InkWell].
Because the Material height is directly related to the visual height. I don't see a place to add margin in this scenario.

we should find way to increase the parent's clip rect so that it won't clip the child

The clip rect enforces the visual height so it is not possible with the given tree.

In #170060, I change the tree and it was possible to add margin but it requires splitting the Material which seems hacky and fragile.
The tree in that PR is:
_DayPeriodInputPadding > Row/Column > [ Semantics > Padding > Material (with shape and clip) > AM InkWell, Semantics > Padding -> Material (with shape and clip) > PM InkWell ]

Do you prefer I close this PR and focus on #170060?

sorry for the late reply, I was busy working on other things in the past few days. I will experiment a bit and get back to you.

After playing a bit, I prefer the approach of the other pr, I think changing the semantics clip behavior and adding renderobject level api is more dangerous.

I suggest we close this pr and continue on the other one, WDYT?

I suggest we close this pr and continue on the other one, WDYT?

Perfect 👍 Let's continue on the other PR.
It will always be time to revisit this if, in the future, we encounter similar problems with other designs that don't match with A11y.

chunhtai · 2025-08-01T19:32:24Z

packages/flutter/lib/src/material/time_picker.dart

+  }
+
+  @override
+  Rect get semanticBounds {


If we increase the SizedBox above the Material widget do we still need this trick?

The core issue is that the visual height is lower than a11y height.
If we increase the SizedBox, it will also increase the visual height.

To be accessible the semantics node associated with each button has to be larger than the Material or the semantics nodes should be defined above the Material (see #170060 where I split the Material into two Material). I also tried to put the semantics nodes in a Stack using another layer for the Material but it led to other issues.

Using a render object that overrides semanticsBounds and adding a flag to escape clipping is the less hacky solution I found for the moment. I restored the commit with this flag, that way you can see the result and compare with the previous commit (the one which change the clipBehavior).

chunhtai · 2025-08-01T19:33:14Z

packages/flutter/test/material/time_picker_test.dart

+          final SemanticsNode amButton = semantics.nodesWith(label: amString).single;
+          expect(amButton.rect.size >= minInteractiveSize, isTrue);
+
+          final SemanticsNode pmButton = semantics.nodesWith(label: pmString).single;


for semantics node can we test through androidTapTargetGuideline?

It was my first thought, but unfortunately the test was passing even without the fix.
I filed an issue: #173208

This reverts commit d0a0dfb.

bleroux · 2025-08-14T08:03:18Z

Closing in favor of #170060.
See #171437 (comment)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 1, 2025

bleroux requested a review from chunhtai July 1, 2025 13:53

chunhtai reviewed Jul 2, 2025

View reviewed changes

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch from 5205aa9 to 69eb1e4 Compare July 3, 2025 14:02

bleroux requested a review from chunhtai July 3, 2025 14:14

flutter-dashboard bot added the will affect goldens Changes to golden files label Jul 3, 2025

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch 3 times, most recently from 79bc84a to 4ef9a54 Compare July 8, 2025 10:17

dkwingsmt requested a review from Piinks July 10, 2025 23:21

Piinks reviewed Jul 17, 2025

View reviewed changes

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch 2 times, most recently from 6abd5e8 to 5833d54 Compare July 21, 2025 06:27

bleroux requested a review from Piinks July 21, 2025 07:14

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch 2 times, most recently from 75f927d to 3818659 Compare July 25, 2025 08:54

chunhtai reviewed Aug 1, 2025

View reviewed changes

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch from 3818659 to bca95bd Compare August 4, 2025 12:54

bleroux requested a review from chunhtai August 5, 2025 06:46

bleroux added 5 commits August 6, 2025 15:01

Fix time picker period selector a11y touch targets

74df259

Fix typo

69eab8e

Fix portrait dial mode padding

27ffcc0

Unset Material.clipBehavior

7453b20

Revert "Unset Material.clipBehavior"

fa3a3f6

This reverts commit d0a0dfb.

bleroux force-pushed the time_picker_day_selector_touch_targets_3 branch from bca95bd to fa3a3f6 Compare August 6, 2025 13:01

bleroux closed this Aug 14, 2025

bleroux deleted the time_picker_day_selector_touch_targets_3 branch August 14, 2025 10:15

bleroux mentioned this pull request Aug 14, 2025

Fix time picker period selector a11y touch targets #170060

Merged

Fix time picker period selector a11y touch targets #171437

Fix time picker period selector a11y touch targets #171437

Uh oh!

Conversation

bleroux commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Implementation choice

Related Issue

Tests

Uh oh!

bleroux commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jul 3, 2025

Uh oh!

flutter-dashboard bot commented Jul 8, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux commented Jul 23, 2025

Uh oh!

flutter-dashboard bot commented Jul 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux commented Aug 14, 2025

Uh oh!

Uh oh!

bleroux commented Jul 1, 2025 •

edited

Loading

bleroux commented Jul 1, 2025 •

edited

Loading

bleroux Jul 25, 2025 •

edited

Loading

bleroux Aug 8, 2025 •

edited

Loading