Skip to content

Commit 9d4db1d

Browse files
blerouxbleroux
authored
Fix SegmentedButton focus issue (flutter#173953)
## 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.
1 parent fe65b1b commit 9d4db1d

File tree

2 files changed

+106
-39
lines changed

2 files changed

+106
-39
lines changed

‎packages/flutter/lib/src/material/segmented_button.dart

Lines changed: 54 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ library;
1111

1212
import 'dart:math' as math;
1313
import 'dart:math';
14+
import 'dart:ui' show lerpDouble;
1415

1516
import 'package:flutter/foundation.dart';
1617
import 'package:flutter/rendering.dart';
@@ -570,44 +571,59 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> {
570571
);
571572
controller.update(WidgetState.selected, segmentSelected);
572573

573-
final Widget button = icon != null
574-
? TextButton.icon(
575-
style: segmentStyle,
576-
statesController: controller,
577-
onHover: (bool hovering) {
578-
setState(() {
579-
_hovering = hovering;
580-
});
581-
},
582-
onFocusChange: (bool focused) {
583-
setState(() {
584-
_focused = focused;
585-
});
586-
},
587-
onPressed: (_enabled && segment.enabled)
588-
? () => _handleOnPressed(segment.value)
589-
: null,
590-
icon: icon,
591-
label: label,
592-
)
593-
: TextButton(
594-
style: segmentStyle,
595-
statesController: controller,
596-
onHover: (bool hovering) {
597-
setState(() {
598-
_hovering = hovering;
599-
});
600-
},
601-
onFocusChange: (bool focused) {
602-
setState(() {
603-
_focused = focused;
604-
});
605-
},
606-
onPressed: (_enabled && segment.enabled)
607-
? () => _handleOnPressed(segment.value)
608-
: null,
609-
child: label,
610-
);
574+
Widget content = label;
575+
ButtonStyle effectiveSegmentStyle = segmentStyle;
576+
if (icon != null) {
577+
// This logic is needed to get the exact same rendering as using TextButton.icon.
578+
// It is duplicated from _TextButtonWithIcon and _TextButtonWithIconChild.
579+
// TODO(bleroux): remove once https://github.com/flutter/flutter/issues/173944 is fixed.
580+
final bool useMaterial3 = Theme.of(context).useMaterial3;
581+
final double defaultFontSize =
582+
segmentStyle.textStyle?.resolve(const <MaterialState>{})?.fontSize ?? 14.0;
583+
final double effectiveTextScale =
584+
MediaQuery.textScalerOf(context).scale(defaultFontSize) / 14.0;
585+
final EdgeInsetsGeometry scaledPadding = ButtonStyleButton.scaledPadding(
586+
useMaterial3
587+
? const EdgeInsetsDirectional.fromSTEB(12, 8, 16, 8)
588+
: const EdgeInsets.all(8),
589+
const EdgeInsets.symmetric(horizontal: 4),
590+
const EdgeInsets.symmetric(horizontal: 4),
591+
effectiveTextScale,
592+
);
593+
effectiveSegmentStyle = segmentStyle.copyWith(
594+
padding: MaterialStatePropertyAll<EdgeInsetsGeometry>(scaledPadding),
595+
);
596+
final double scale = clampDouble(effectiveTextScale, 1.0, 2.0) - 1.0;
597+
final TextButtonThemeData textButtonTheme = TextButtonTheme.of(context);
598+
final IconAlignment effectiveIconAlignment =
599+
textButtonTheme.style?.iconAlignment ??
600+
segmentStyle.iconAlignment ??
601+
IconAlignment.start;
602+
content = Row(
603+
mainAxisSize: MainAxisSize.min,
604+
spacing: lerpDouble(8, 4, scale)!,
605+
children: effectiveIconAlignment == IconAlignment.start
606+
? <Widget>[icon, Flexible(child: label)]
607+
: <Widget>[Flexible(child: label), icon],
608+
);
609+
}
610+
611+
final Widget button = TextButton(
612+
style: effectiveSegmentStyle,
613+
statesController: controller,
614+
onHover: (bool hovering) {
615+
setState(() {
616+
_hovering = hovering;
617+
});
618+
},
619+
onFocusChange: (bool focused) {
620+
setState(() {
621+
_focused = focused;
622+
});
623+
},
624+
onPressed: (_enabled && segment.enabled) ? () => _handleOnPressed(segment.value) : null,
625+
child: content,
626+
);
611627

612628
final Widget buttonWithTooltip = segment.tooltip != null
613629
? Tooltip(message: segment.tooltip, child: button)

‎packages/flutter/test/material/segmented_button_test.dart

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,57 @@ void main() {
243243
expect(selection, <int>{2, 3});
244244
});
245245

246+
// Regression test for https://github.com/flutter/flutter/issues/161922.
247+
testWidgets('Focused segment does not lose focus when its selection state changes', (
248+
WidgetTester tester,
249+
) async {
250+
int callbackCount = 0;
251+
Set<int> selection = <int>{1};
252+
253+
Widget frameWithSelection(Set<int> selected) {
254+
return Material(
255+
child: boilerplate(
256+
child: SegmentedButton<int>(
257+
multiSelectionEnabled: true,
258+
segments: const <ButtonSegment<int>>[
259+
ButtonSegment<int>(value: 1, label: Text('1')),
260+
ButtonSegment<int>(value: 2, label: Text('2')),
261+
],
262+
selected: selected,
263+
onSelectionChanged: (Set<int> selected) {
264+
selection = selected;
265+
callbackCount += 1;
266+
},
267+
),
268+
),
269+
);
270+
}
271+
272+
await tester.pumpWidget(frameWithSelection(selection));
273+
expect(selection, <int>{1});
274+
expect(callbackCount, 0);
275+
276+
// Select segment 2.
277+
await tester.pumpWidget(frameWithSelection(<int>{1, 2}));
278+
await tester.pumpAndSettle();
279+
280+
FocusNode getSegment2FocusNode() {
281+
return Focus.of(tester.element(find.text('2')));
282+
}
283+
284+
// Set focus on segment 2.
285+
getSegment2FocusNode().requestFocus();
286+
await tester.pumpAndSettle();
287+
expect(getSegment2FocusNode().hasFocus, true);
288+
289+
// Unselect segment 2.
290+
await tester.pumpWidget(frameWithSelection(<int>{1}));
291+
await tester.pumpAndSettle();
292+
293+
// The button should still be focused.
294+
expect(getSegment2FocusNode().hasFocus, true);
295+
});
296+
246297
testWidgets('SegmentedButton allows for empty selection', (WidgetTester tester) async {
247298
int callbackCount = 0;
248299
int? selectedSegment = 1;
@@ -585,7 +636,7 @@ void main() {
585636
);
586637

587638
final Material material = tester.widget<Material>(
588-
find.descendant(of: find.byType(TextButton), matching: find.byType(Material)),
639+
find.descendant(of: find.byType(TextButton).last, matching: find.byType(Material)),
589640
);
590641

591642
// Hovered.

0 commit comments

Comments
 (0)