-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Migrate CalendarDatePicker to declarative live regions for accessibility #180131
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
base: master
Are you sure you want to change the base?
Migrate CalendarDatePicker to declarative live regions for accessibility #180131
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 successfully migrates CalendarDatePicker and _MonthPicker from the deprecated SemanticsService.sendAnnouncement to a declarative approach using Semantics with liveRegion: true. The changes are well-implemented, replacing imperative announcements with state updates that drive a live region for accessibility announcements.
My main feedback is on the test file calendar_date_picker_test.dart. While the test was correctly updated to reflect the new implementation, I found the logic for verifying semantics announcements to be brittle and contain a subtle bug. I've provided a suggestion to refactor the test for improved robustness and readability.
Overall, this is a solid improvement for accessibility compliance.
|
Hi @Franklyn-R-Silva, |
…icker - Wrap Semantics live region with SizedBox and Column to avoid zero-size nodes. - Add accessiblityFocusBlockType to ensure correct focus traversal. - Maintain imperative announcements for non-Android platforms in _handleDayChanged and _handleMonthPageChanged.
Hi @hannah-hyj, thank you for the feedback! I have updated the code to address the accessibility concerns: Visibility: I wrapped the Semantics live regions with widgets that have physical dimensions (SizedBox and Column) to ensure they are not dropped from the semantics tree. Focus Traversal: I added accessiblityFocusBlockType: AccessiblityFocusBlockType.none to maintain correct traversal after adding the live regions. Platform Parity: I refactored the announcement logic to ensure Android uses the declarative approach while maintaining imperative announcements for other platforms. Note: I am still working on updating the tests in calendar_date_picker_test.dart to reflect these changes and will push them shortly. I will ping you again once the tests are ready for review. |
Hi @hannah-hyj, I’ve updated the PR to address your feedback: Visibility: Wrapped Semantics in visible containers (SizedBox/Column) to prevent zero-size nodes. Focus: Added accessiblityFocusBlockType to ensure correct traversal. Platforms: Refactored logic to use declarative live regions on Android and maintain imperative announcements on other platforms. Tests: Updated tests using the Android variant to verify the isLiveRegion flag. Ready for another look! |
hannah-hyj
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.
Hi @Franklyn-R-Silva I just got back from the vacation and I'm taking a look at this PR again!
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Franklyn-R-Silva
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.
review
| return Stack( | ||
| children: <Widget>[ | ||
| SizedBox(height: _subHeaderHeight + scaledMaxDayPickerHeight, child: _buildPicker()), | ||
| Semantics( |
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.
we should only add this Semantics widget here if platform == TargetPlatform.android.
| '${_localizations.selectedDateLabel} ${widget.calendarDelegate.formatFullDate(_selectedDate!, _localizations)}$semanticLabelSuffix', | ||
| _textDirection, | ||
| ); | ||
| case TargetPlatform.android: |
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.
It looks like the previous behavior was to use sendAnnouncement only on Linux, macOS, and Windows, but not on Android or iOS. I'm not sure why that was the case. However, since the goal of this PR is to remove the deprecated sendAnnouncement usage on Android, I don't think we need to change anything in this code block (L313–L328).
|
Hi @Franklyn-R-Silva ! this PR looks good to me over all and i just left a few more comments! |
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
Co-authored-by: Hannah Jin <jhy03261997@gmail.com>
|
looks like some tests in calendar_date_picker_test.dart is failing, could you take a look? |
Description
This PR migrates the
CalendarDatePickerand_MonthPickerwidgets from the deprecatedSemanticsService.sendAnnouncementto a declarative approach usingliveRegion: true.As specified in #180096 and #177785,
SemanticsService.sendAnnouncementis deprecated on Android as of API 36. By using a live region, we ensure that state change announcements (like month navigation and date selection) remain accessible and compliant with modern platform standards.Changes:
_announcementTextto_CalendarDatePickerStateand_MonthPickerStateto track accessibility messages.Semanticswidget withliveRegion: truein the build methods to process these announcements.calendar_date_picker_test.dartto verify theSemanticsFlag.isLiveRegionon the semantics tree instead of capturing imperatice announcements.Related Issues
Fixes #177785
Relates to #180096
Tests
packages/flutter/test/material/calendar_date_picker_test.dartto verify the new declarative announcement logic.