-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Fix SegmentedButton focus issue #173953
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 SegmentedButton focus issue #173953
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 effectively addresses a focus traversal issue in the SegmentedButton widget. The core of the fix is to refactor the button segment's construction to always use TextButton, manually building the child content to ensure a consistent widget tree, which prevents focus from being lost when a segment's selection state changes. The implementation choice to duplicate logic from private TextButton classes is well-justified as a necessary workaround and is clearly documented with a TODO. The accompanying regression test is a great addition that verifies the fix. My feedback is minor, focusing on improving the clarity of a test name for better long-term maintainability.
bc7d98a to
1aba614
Compare
QuncCccccc
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!
| const EdgeInsets.symmetric(horizontal: 4), | ||
| effectiveTextScale, | ||
| ); | ||
| effectiveSegmentStyle = segmentStyle.copyWith( |
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.
Oh nice, I didn't notice we need to check M2 and M3 text button style:)
…9862) Manual roll Flutter from e65380a22076 to 960d1078f876 (36 revisions) Manual roll requested by [email protected] flutter/flutter@e65380a...960d107 2025-08-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reapply "Add set semantics enabled API and wire iOS a11y bridge (#161… (#171198)" (flutter/flutter#174153) 2025-08-20 [email protected] Make sure that a Badge doesn't crash in 0x0 environment (flutter/flutter#172065) 2025-08-20 [email protected] Make sure that CalendarDatePicker & YearPicker don't crash in 0x0 environment (flutter/flutter#173408) 2025-08-20 [email protected] Roll Packages from 953cae0 to 58c02e0 (2 revisions) (flutter/flutter#174142) 2025-08-20 [email protected] Make sure that a CircleAvatar doesn't crash in 0x0 environment (flutter/flutter#173498) 2025-08-20 [email protected] Roll Dart SDK from 0d674ff61e2e to 0d0a0c394381 (1 revision) (flutter/flutter#174126) 2025-08-20 [email protected] [Android] Fix version code override calculation in FlutterPlugin (flutter/flutter#174081) 2025-08-20 [email protected] Make sure that a BackButton doesn't crash in 0x0 environment (flutter/flutter#172817) 2025-08-20 [email protected] Roll Dart SDK from c5f5a32df36c to 0d674ff61e2e (1 revision) (flutter/flutter#174099) 2025-08-20 [email protected] feat: Added FocusNode prop for DropdownMenu Trailing Icon Button (flutter/flutter#172753) 2025-08-20 [email protected] Make component theme data defaults use `WidgetStateProperty` (flutter/flutter#173893) 2025-08-20 [email protected] Fix Menu anchor reduce padding on web and desktop (flutter/flutter#172691) 2025-08-20 [email protected] Roll Skia from 4b788d0e5e63 to 721e68fe652a (2 revisions) (flutter/flutter#174095) 2025-08-20 [email protected] Fix time picker period selector a11y touch targets (flutter/flutter#170060) 2025-08-20 [email protected] Fix SegmentedButton focus issue (flutter/flutter#173953) 2025-08-20 [email protected] Roll Dart SDK from e936404543f1 to c5f5a32df36c (1 revision) (flutter/flutter#174089) 2025-08-20 [email protected] Roll Skia from 953bfc0e2f2a to 4b788d0e5e63 (1 revision) (flutter/flutter#174086) 2025-08-19 [email protected] Roll Skia from 07d71ea4d056 to 953bfc0e2f2a (18 revisions) (flutter/flutter#174072) 2025-08-19 [email protected] Roll Dart SDK from 9105d946af95 to e936404543f1 (5 revisions) (flutter/flutter#174074) 2025-08-19 [email protected] NavigationRail correct traversal order (flutter/flutter#173891) 2025-08-19 [email protected] Update CupertinoSliverNavigationBar.middle (flutter/flutter#173868) 2025-08-19 [email protected] Update the AccessibilityPlugin::Announce method to account for the view (flutter/flutter#172669) 2025-08-19 [email protected] [ Widget Preview ] Report an error if a web device is unavailable (flutter/flutter#174036) 2025-08-19 [email protected] [web] Fix error in ClickDebouncer when using VoiceOver (flutter/flutter#174046) 2025-08-19 [email protected] [ Tool ] Add logging to test_adapter_test.dart (flutter/flutter#174073) 2025-08-19 [email protected] Roll Fuchsia Linux SDK from n0EnLlotF2wczlOq_... to V1A1J6uXZ62Q10i9u... (flutter/flutter#174059) 2025-08-19 [email protected] Cleanup legacy `bringup: true` tasks, either removing or enabling (flutter/flutter#173815) 2025-08-19 [email protected] Add Shift+Enter shortcut example for TextField. (flutter/flutter#167952) 2025-08-19 [email protected] Check that the windows architecture is 64-bit and not the process architecture (flutter/flutter#174019) 2025-08-19 [email protected] Improve Stack widget error message for bounded constraints (flutter/flutter#173352) 2025-08-19 [email protected] [VPAT][A11y] AutoComplete dropdown option is missing button role (flutter/flutter#173297) 2025-08-19 [email protected] fix: Android build fails when minSdk is set below 24 in build.gradle.kts (#173823) (flutter/flutter#173825) 2025-08-19 [email protected] Reapply "Add set semantics enabled API and wire iOS a11y bridge (#161… (flutter/flutter#171198) 2025-08-19 [email protected] fix: only use library props for libraries (flutter/flutter#172704) 2025-08-19 [email protected] Roll Packages from 5c52c55 to 953cae0 (22 revisions) (flutter/flutter#174040) 2025-08-19 [email protected] Add `open_jdk` to `Linux linux_android_emulator.debug_x64` (flutter/flutter#173989) 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 [email protected],[email protected] 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 ...
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
## Description This PR fixes SegmentedButton focus traversal. # Before When a focused segment (with no icon) is selected or unselected, the focus moves to another segment. [Capture vidéo du 2025-08-11 15-28-03.webm](https://github.com/user-attachments/assets/11c29194-6b96-4ea4-9245-dcbd36dc424f) In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment. # After When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted. [Capture vidéo du 2025-08-11 15-27-30.webm](https://github.com/user-attachments/assets/e09056d0-b572-462b-8ad8-c23eddcc4bfd) <details><summary>Code sample for recordings</summary> ```dart import 'package:flutter/material.dart'; /// Flutter code sample for [SegmentedButton]. void main() { runApp(const SegmentedButtonApp()); } class SegmentedButtonApp extends StatelessWidget { const SegmentedButtonApp({super.key}); @OverRide Widget build(BuildContext context) { return const MaterialApp( home: Scaffold(body: Center(child: SingleChoice())), ); } } enum Calendar { day, week, month, year } class SingleChoice extends StatefulWidget { const SingleChoice({super.key}); @OverRide State<SingleChoice> createState() => _SingleChoiceState(); } class _SingleChoiceState extends State<SingleChoice> { Calendar calendarView = Calendar.day; @OverRide Widget build(BuildContext context) { return SegmentedButton<Calendar>( segments: const <ButtonSegment<Calendar>>[ ButtonSegment<Calendar>(value: Calendar.day, label: Text('Day')), ButtonSegment<Calendar>(value: Calendar.week, label: Text('Week')), ButtonSegment<Calendar>(value: Calendar.month, label: Text('Month')), ButtonSegment<Calendar>(value: Calendar.year, label: Text('Year')), ], selected: <Calendar>{calendarView}, onSelectionChanged: (Set<Calendar> newSelection) { setState(() { // By default there is only a single segment that can be // selected at one time, so its value is always the first // item in the selected set. calendarView = newSelection.first; }); }, ); } } ``` </details> ## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](flutter#161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See flutter#173944 for more context. The proposed solution is to maintain the same widget tree when possible. To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label. In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated. ## Tests Adds 1 test. Updates 1 test.
Description
This PR fixes SegmentedButton focus traversal.
Before
When a focused segment (with no icon) is selected or unselected, the focus moves to another segment.
Capture.video.du.2025-08-11.15-28-03.webm
In this recording, tab key is used to set the focus on 'Month' segment, then enter is used to select the segment. The focus move unexpectedly to 'Week' segment.
After
When a focused segment (with no icon) is selected or unselected, the focus stays on this same segment and the InkWell animations are correctly painted.
Capture.video.du.2025-08-11.15-27-30.webm
Code sample for recordings
Related Issue
Fixes SegmentedButton keyboard navigation incorrectly moves focus on click
Implementation choice
The root of this issue is TextButton.icon which creates a different widget tree depending on the icon.
See #173944 for more context.
The proposed solution is to maintain the same widget tree when possible.
To do TextButton.icon is not used. This PR uses on TextButton and build a child containing both the icon (when present) and the label.
In order to keep the same rendering as before, some internal logic from TextButton.icon is duplicated.
Tests
Adds 1 test.
Updates 1 test.