-
Notifications
You must be signed in to change notification settings - Fork 29.8k
fix: exclude semantics for disabled dates #178981
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: exclude semantics for disabled dates #178981
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a fix to exclude semantics for disabled dates in CupertinoDatePicker. The changes involve wrapping disabled date/time items with the ExcludeSemantics widget across various picker builders. This is complemented by a modification in _RenderCupertinoPickerSemantics to correctly handle items with empty labels, preventing accessibility actions from navigating to disabled items. A new widget test has been added to verify that disabled dates are indeed excluded from semantics, ensuring that accessibility tools like screen readers do not announce or interact with them. The implementation is consistent and well-tested, effectively addressing the issue.
ed4797a to
5273ad3
Compare
| } | ||
| config.value = indexedChildren[_currentIndex]!.label; | ||
| final String currentLabel = indexedChildren[_currentIndex]!.label; | ||
| // If the current item has an empty label (e.g., wrapped with ExcludeSemantics), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is wrapped in ExcludeSemantics how does this method get called in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @chunhtai,
If we look at assembleSemanticsNode in picker.dart:553:
- The method iterates over all child semantics nodes via scrollable.visitChildren
- It builds indexedChildren map using each node's indexInParent
- Nodes wrapped with ExcludeSemantics are still present in this iteration - they just have an empty label
So the SemanticsNode for a disabled date still exists and gets added to indexedChildren, but its label property is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it is the ListWheelChildBuilderDelegate that wrap all the children with indexedSemantics even for those who ExcludedSemantics which is not ideal, but fixing this will probably need to modify the API which probably not worth it. This is good then
| } | ||
| config.value = currentLabel; | ||
| final SemanticsNode? previousChild = indexedChildren[_currentIndex - 1]; | ||
| final SemanticsNode? nextChild = indexedChildren[_currentIndex + 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, for the ExludesSemantics, they shouldn't be in the in indexedChildren I think? is the ExludesSemantics wrapped in the wrong level?
|
What is the status of this PR? (from triage) |
Waiting for another review after #178981 (comment) |
chunhtai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| } | ||
| config.value = indexedChildren[_currentIndex]!.label; | ||
| final String currentLabel = indexedChildren[_currentIndex]!.label; | ||
| // If the current item has an empty label (e.g., wrapped with ExcludeSemantics), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it is the ListWheelChildBuilderDelegate that wrap all the children with indexedSemantics even for those who ExcludedSemantics which is not ideal, but fixing this will probably need to modify the API which probably not worth it. This is good then
| // The semantics system requires that if "value" is empty, "increasedValue" | ||
| // and "decreasedValue" must also be empty, and no increase/decrease actions | ||
| // should be set. | ||
| if (currentLabel.isEmpty) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also add a test for this if-block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will add and push.
5273ad3 to
6e86c0c
Compare
6e86c0c to
c6f64ae
Compare
Roll Flutter from 6ff7f30 to 60d8165 (32 revisions) flutter/flutter@6ff7f30...60d8165 2025-12-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from xn1U9DUFEpB4f6vG-... to DdllqZRZYriOd7Q8v... (flutter/flutter#180356) 2025-12-28 engine-flutter-autoroll@skia.org Roll Dart SDK from 17d51009282c to df175c998021 (1 revision) (flutter/flutter#180349) 2025-12-28 engine-flutter-autoroll@skia.org Roll Dart SDK from d4a92c2c69bf to 17d51009282c (1 revision) (flutter/flutter#180346) 2025-12-28 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from jVkEEUxyGeemuu2HE... to xn1U9DUFEpB4f6vG-... (flutter/flutter#180343) 2025-12-27 engine-flutter-autoroll@skia.org Roll Dart SDK from 3b84125003a1 to d4a92c2c69bf (1 revision) (flutter/flutter#180331) 2025-12-27 engine-flutter-autoroll@skia.org Roll Skia from e6e535aec613 to c29a475066f5 (1 revision) (flutter/flutter#180322) 2025-12-27 ahmedsameha1@gmail.com Make sure that a WidgetsApp doesn't crash in 0x0 environment (flutter/flutter#180224) 2025-12-27 ahmedsameha1@gmail.com Make sure that an AnimatedSize doesn't crash in 0x0 environment (flutter/flutter#180174) 2025-12-27 ahmedsameha1@gmail.com Make sure that an AnimatedGrid doesn't crash in 0x0 environment (flutter/flutter#180159) 2025-12-27 ahmedsameha1@gmail.com Make sure that an AnimatedCrossFade does not crash in 0x0 environment (flutter/flutter#180088) 2025-12-27 ahmedsameha1@gmail.com Make sure that an AnimatedList doesn't crash in 0x0 environment (flutter/flutter#180123) 2025-12-26 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from mZlbuHta7S71Quawk... to jVkEEUxyGeemuu2HE... (flutter/flutter#180317) 2025-12-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 6b2c3716be54 to 3b84125003a1 (1 revision) (flutter/flutter#180316) 2025-12-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 18f6751c5ccc to 6b2c3716be54 (1 revision) (flutter/flutter#180252) 2025-12-26 engine-flutter-autoroll@skia.org Roll Skia from 07c59737d265 to e6e535aec613 (1 revision) (flutter/flutter#180303) 2025-12-25 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 2q71ydbnIhmBUiNOF... to mZlbuHta7S71Quawk... (flutter/flutter#180290) 2025-12-25 engine-flutter-autoroll@skia.org Roll Skia from 6d0a1b03e9c7 to 07c59737d265 (1 revision) (flutter/flutter#180289) 2025-12-25 engine-flutter-autoroll@skia.org Roll Skia from 2b5d93b62a3f to 6d0a1b03e9c7 (1 revision) (flutter/flutter#180279) 2025-12-25 engine-flutter-autoroll@skia.org Roll Skia from 3544942c9d42 to 2b5d93b62a3f (1 revision) (flutter/flutter#180276) 2025-12-24 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5EgkVbjGVZmCFPdtR... to 2q71ydbnIhmBUiNOF... (flutter/flutter#180274) 2025-12-24 engine-flutter-autoroll@skia.org Roll Packages from 5e3a766 to 2164da9 (4 revisions) (flutter/flutter#180271) 2025-12-24 engine-flutter-autoroll@skia.org Roll Skia from 988a48f7a944 to 3544942c9d42 (1 revision) (flutter/flutter#180267) 2025-12-24 fluttergithubbot@gmail.com Marks Mac_arm64_mokey native_assets_android to be unflaky (flutter/flutter#180263) 2025-12-24 engine-flutter-autoroll@skia.org Roll Skia from 51a05ce05880 to 988a48f7a944 (1 revision) (flutter/flutter#180256) 2025-12-24 34465683+rkishan516@users.noreply.github.com fix: exclude semantics for disabled dates (flutter/flutter#178981) 2025-12-24 engine-flutter-autoroll@skia.org Roll Skia from 7fe9b93153f8 to 51a05ce05880 (1 revision) (flutter/flutter#180253) 2025-12-24 116356835+AbdeMohlbi@users.noreply.github.com Enhance documentation for sortable column behavior (flutter/flutter#180011) 2025-12-24 engine-flutter-autoroll@skia.org Roll Skia from a0f18947bc1b to 7fe9b93153f8 (4 revisions) (flutter/flutter#180247) 2025-12-23 engine-flutter-autoroll@skia.org Roll Skia from bdb147ae3040 to a0f18947bc1b (2 revisions) (flutter/flutter#180241) 2025-12-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 94b05f717ba3 to 18f6751c5ccc (1 revision) (flutter/flutter#180236) 2025-12-23 kazbek.sultanov.doc@gmail.com docs: clarify dropdown menu text clearing (flutter/flutter#179676) 2025-12-23 flar@google.com New optimized general convex path shadow algorithm (flutter/flutter#178370) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...
Changes
fixes: #178713
Pre-launch Checklist
///).