Fix SegmentedButton focus issue (#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](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.
This commit is contained in:
Bruno Leroux 2025-08-20 08:10:07 +02:00 committed by GitHub
parent fe65b1b90a
commit 9d4db1d5dd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 106 additions and 39 deletions

View File

@ -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<T> extends State<SegmentedButton<T>> {
);
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 <MaterialState>{})?.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<EdgeInsetsGeometry>(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
? <Widget>[icon, Flexible(child: label)]
: <Widget>[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)

View File

@ -243,6 +243,57 @@ void main() {
expect(selection, <int>{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<int> selection = <int>{1};
Widget frameWithSelection(Set<int> selected) {
return Material(
child: boilerplate(
child: SegmentedButton<int>(
multiSelectionEnabled: true,
segments: const <ButtonSegment<int>>[
ButtonSegment<int>(value: 1, label: Text('1')),
ButtonSegment<int>(value: 2, label: Text('2')),
],
selected: selected,
onSelectionChanged: (Set<int> selected) {
selection = selected;
callbackCount += 1;
},
),
),
);
}
await tester.pumpWidget(frameWithSelection(selection));
expect(selection, <int>{1});
expect(callbackCount, 0);
// Select segment 2.
await tester.pumpWidget(frameWithSelection(<int>{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(<int>{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<Material>(
find.descendant(of: find.byType(TextButton), matching: find.byType(Material)),
find.descendant(of: find.byType(TextButton).last, matching: find.byType(Material)),
);
// Hovered.