Repository navigation
Migrate CalendarDatePicker to declarative live regions for accessibility - #180131
Franklyn-R-Silva wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
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.
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 <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
| return Stack( | ||
| children: <Widget>[ | ||
| SizedBox(height: _subHeaderHeight + scaledMaxDayPickerHeight, child: _buildPicker()), | ||
| Semantics( |
There was a problem hiding this comment.
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.
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 <[email protected]>
Co-authored-by: Hannah Jin <[email protected]>
|
looks like some tests in calendar_date_picker_test.dart is failing, could you take a look? |
Yes, I'll check it out. I'm very busy at work and haven't had time to check everything yet. |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@Franklyn-R-Silva Do you still plan to come back and finish this PR? |
|
hi @Franklyn-R-Silva, thank you for your attention to this issue. I will now mark this PR as inactive and close it, and create a new PR based on it. |
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.