Skip to content

Fix SegmentedButton border doesn't reflect states #172754

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Jul 25, 2025

Description

This PR fixes SegmentedButton border side not depending on the current state.

It's based on one @TahaTesser 's great PR: see #161942. 🙏
I updated some logic related to disabled border which led to Taha’s PR failing internal Google tests.
I also added a test to verify the disabled border.

Code sample for screenshots
import 'package:flutter/material.dart';

void main() {
  runApp(const SegmentedButtonApp());
}

enum Calendar { day, week, month, year }

class SegmentedButtonApp extends StatefulWidget {
  const SegmentedButtonApp({super.key});

  @override
  State<SegmentedButtonApp> createState() => _SegmentedButtonAppState();
}

class _SegmentedButtonAppState extends State<SegmentedButtonApp> {
  Calendar? calendarView;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        segmentedButtonTheme: SegmentedButtonThemeData(
          style: ButtonStyle(
            side: WidgetStateProperty.fromMap(<WidgetStatesConstraint, BorderSide?>{
              WidgetState.disabled: const BorderSide(color: Colors.grey, width: 2),
              WidgetState.selected & WidgetState.hovered: const BorderSide(
                color: Colors.blue,
                width: 2,
              ),
              WidgetState.selected & WidgetState.focused: const BorderSide(
                color: Colors.pinkAccent,
                width: 2,
              ),
              WidgetState.hovered: const BorderSide(color: Colors.green, width: 2),
              WidgetState.focused: const BorderSide(color: Colors.purple, width: 2),
              WidgetState.any: const BorderSide(color: Colors.amber, width: 2),
            }),
          ),
        ),
      ),
      home: Scaffold(
        body: Center(
          child: SegmentedButton<Calendar>(
            segments: const <ButtonSegment<Calendar>>[
              ButtonSegment<Calendar>(
                value: Calendar.day,
                label: Text('Day'),
                icon: Icon(Icons.calendar_view_day),
                enabled: false,
              ),
              ButtonSegment<Calendar>(
                value: Calendar.week,
                label: Text('Week'),
                icon: Icon(Icons.calendar_view_week),
              ),
              ButtonSegment<Calendar>(
                value: Calendar.month,
                label: Text('Month'),
                icon: Icon(Icons.calendar_view_month),
              ),
              ButtonSegment<Calendar>(
                value: Calendar.year,
                label: Text('Year'),
                icon: Icon(Icons.calendar_today),
              ),
            ],
            selected: <Calendar>{?calendarView},
            emptySelectionAllowed: true,
            onSelectionChanged: (Set<Calendar> newSelection) {
              setState(() {
                calendarView = newSelection.isEmpty ? null : newSelection.first;
              });
            },
          ),
        ),
        floatingActionButton: FloatingActionButton(onPressed: () {}, child: const Icon(Icons.add)),
      ),
    );
  }
}

Before

Screen.Recording.2025-07-25.at.10.40.40.mov

After

Screen.Recording.2025-07-25.at.10.39.33.mov

Related Issue

Fixes SegmentedButton does not set its MaterialState for side

Tests

Adds 2 tests.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 25, 2025
@bleroux
Copy link
Contributor Author

bleroux commented Jul 25, 2025

@QuncCccccc @victorsanni As you reviewed the initial PR (#172754), can you please review this one?

expect(find.byType(SegmentedButton<int>), paints..rrect(color: selectedColor));
});

testWidgets('SegmentedButton border sides respect disabled state', (WidgetTester tester) async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test passes on master, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this test was added to catch future regression such as the one #172754 encountered. In the case of #172754, the regression was caught thanks to Google tests failures.
In summary, it is a missing test that should have been there since the creation of SegmentedButton.

@bleroux bleroux requested a review from victorsanni July 25, 2025 19:32
@bleroux bleroux force-pushed the fix_segmented_button_side_states branch from fd8c596 to c93a982 Compare July 28, 2025 06:49
@bleroux
Copy link
Contributor Author

bleroux commented Jul 28, 2025

@victorsanni Thanks for the review 🙏
Do you have a way to trigger the Google tests? (Usually it runs automatically after the approval, looks like it does not in this case). As the previous PR was failing Google tests, I think it would be better to see if this one passes those tests before adding autosubmit label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SegmentedButton does not set its MaterialState for side
2 participants