From 9d4db1d5dd5682dcb459b3a05ae4e16d102e1e80 Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Wed, 20 Aug 2025 08:10:07 +0200 Subject: [PATCH] Fix SegmentedButton focus issue (#173953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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)
Code sample for recordings ```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 createState() => _SingleChoiceState(); } class _SingleChoiceState extends State { Calendar calendarView = Calendar.day; @override Widget build(BuildContext context) { return SegmentedButton( segments: const >[ ButtonSegment(value: Calendar.day, label: Text('Day')), ButtonSegment(value: Calendar.week, label: Text('Week')), ButtonSegment(value: Calendar.month, label: Text('Month')), ButtonSegment(value: Calendar.year, label: Text('Year')), ], selected: {calendarView}, onSelectionChanged: (Set 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; }); }, ); } } ```
## Related Issue Fixes [SegmentedButton keyboard navigation incorrectly moves focus on click](https://github.com/flutter/flutter/issues/161922) ## Implementation choice The root of this issue is TextButton.icon which creates a different widget tree depending on the icon. See https://github.com/flutter/flutter/issues/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. --- .../lib/src/material/segmented_button.dart | 92 +++++++++++-------- .../test/material/segmented_button_test.dart | 53 ++++++++++- 2 files changed, 106 insertions(+), 39 deletions(-) diff --git a/packages/flutter/lib/src/material/segmented_button.dart b/packages/flutter/lib/src/material/segmented_button.dart index 9bf7ec23fd7..aa231afd1cf 100644 --- a/packages/flutter/lib/src/material/segmented_button.dart +++ b/packages/flutter/lib/src/material/segmented_button.dart @@ -11,6 +11,7 @@ library; import 'dart:math' as math; import 'dart:math'; +import 'dart:ui' show lerpDouble; import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; @@ -570,44 +571,59 @@ class SegmentedButtonState extends State> { ); controller.update(WidgetState.selected, segmentSelected); - final Widget button = icon != null - ? TextButton.icon( - style: segmentStyle, - statesController: controller, - onHover: (bool hovering) { - setState(() { - _hovering = hovering; - }); - }, - onFocusChange: (bool focused) { - setState(() { - _focused = focused; - }); - }, - onPressed: (_enabled && segment.enabled) - ? () => _handleOnPressed(segment.value) - : null, - icon: icon, - label: label, - ) - : TextButton( - style: segmentStyle, - statesController: controller, - onHover: (bool hovering) { - setState(() { - _hovering = hovering; - }); - }, - onFocusChange: (bool focused) { - setState(() { - _focused = focused; - }); - }, - onPressed: (_enabled && segment.enabled) - ? () => _handleOnPressed(segment.value) - : null, - child: label, - ); + Widget content = label; + ButtonStyle effectiveSegmentStyle = segmentStyle; + if (icon != null) { + // This logic is needed to get the exact same rendering as using TextButton.icon. + // It is duplicated from _TextButtonWithIcon and _TextButtonWithIconChild. + // TODO(bleroux): remove once https://github.com/flutter/flutter/issues/173944 is fixed. + final bool useMaterial3 = Theme.of(context).useMaterial3; + final double defaultFontSize = + segmentStyle.textStyle?.resolve(const {})?.fontSize ?? 14.0; + final double effectiveTextScale = + MediaQuery.textScalerOf(context).scale(defaultFontSize) / 14.0; + final EdgeInsetsGeometry scaledPadding = ButtonStyleButton.scaledPadding( + useMaterial3 + ? const EdgeInsetsDirectional.fromSTEB(12, 8, 16, 8) + : const EdgeInsets.all(8), + const EdgeInsets.symmetric(horizontal: 4), + const EdgeInsets.symmetric(horizontal: 4), + effectiveTextScale, + ); + effectiveSegmentStyle = segmentStyle.copyWith( + padding: MaterialStatePropertyAll(scaledPadding), + ); + final double scale = clampDouble(effectiveTextScale, 1.0, 2.0) - 1.0; + final TextButtonThemeData textButtonTheme = TextButtonTheme.of(context); + final IconAlignment effectiveIconAlignment = + textButtonTheme.style?.iconAlignment ?? + segmentStyle.iconAlignment ?? + IconAlignment.start; + content = Row( + mainAxisSize: MainAxisSize.min, + spacing: lerpDouble(8, 4, scale)!, + children: effectiveIconAlignment == IconAlignment.start + ? [icon, Flexible(child: label)] + : [Flexible(child: label), icon], + ); + } + + final Widget button = TextButton( + style: effectiveSegmentStyle, + statesController: controller, + onHover: (bool hovering) { + setState(() { + _hovering = hovering; + }); + }, + onFocusChange: (bool focused) { + setState(() { + _focused = focused; + }); + }, + onPressed: (_enabled && segment.enabled) ? () => _handleOnPressed(segment.value) : null, + child: content, + ); final Widget buttonWithTooltip = segment.tooltip != null ? Tooltip(message: segment.tooltip, child: button) diff --git a/packages/flutter/test/material/segmented_button_test.dart b/packages/flutter/test/material/segmented_button_test.dart index a540ec71fae..ec39eaa0c24 100644 --- a/packages/flutter/test/material/segmented_button_test.dart +++ b/packages/flutter/test/material/segmented_button_test.dart @@ -243,6 +243,57 @@ void main() { expect(selection, {2, 3}); }); + // Regression test for https://github.com/flutter/flutter/issues/161922. + testWidgets('Focused segment does not lose focus when its selection state changes', ( + WidgetTester tester, + ) async { + int callbackCount = 0; + Set selection = {1}; + + Widget frameWithSelection(Set selected) { + return Material( + child: boilerplate( + child: SegmentedButton( + multiSelectionEnabled: true, + segments: const >[ + ButtonSegment(value: 1, label: Text('1')), + ButtonSegment(value: 2, label: Text('2')), + ], + selected: selected, + onSelectionChanged: (Set selected) { + selection = selected; + callbackCount += 1; + }, + ), + ), + ); + } + + await tester.pumpWidget(frameWithSelection(selection)); + expect(selection, {1}); + expect(callbackCount, 0); + + // Select segment 2. + await tester.pumpWidget(frameWithSelection({1, 2})); + await tester.pumpAndSettle(); + + FocusNode getSegment2FocusNode() { + return Focus.of(tester.element(find.text('2'))); + } + + // Set focus on segment 2. + getSegment2FocusNode().requestFocus(); + await tester.pumpAndSettle(); + expect(getSegment2FocusNode().hasFocus, true); + + // Unselect segment 2. + await tester.pumpWidget(frameWithSelection({1})); + await tester.pumpAndSettle(); + + // The button should still be focused. + expect(getSegment2FocusNode().hasFocus, true); + }); + testWidgets('SegmentedButton allows for empty selection', (WidgetTester tester) async { int callbackCount = 0; int? selectedSegment = 1; @@ -585,7 +636,7 @@ void main() { ); final Material material = tester.widget( - find.descendant(of: find.byType(TextButton), matching: find.byType(Material)), + find.descendant(of: find.byType(TextButton).last, matching: find.byType(Material)), ); // Hovered.