Skip to content
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 border doesn't respect hovered, focused, selected states #161942

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

TahaTesser
Copy link
Member

Fixes SegmentedButton does not set its MaterialState for side

Code Sample

expand to view the code sample
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 = Calendar.day;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        segmentedButtonTheme: SegmentedButtonThemeData(
          style: ButtonStyle(
            side: WidgetStateProperty.resolveWith(
              (Set<WidgetState> states) {
                if (states.contains(WidgetState.disabled)) {
                  return const BorderSide(color: Colors.grey, width: 2);
                }
                if (states.contains(WidgetState.hovered)) {
                  return const BorderSide(color: Colors.green, width: 2);
                }
                if (states.contains(WidgetState.focused)) {
                  return const BorderSide(color: Colors.pinkAccent, width: 2);
                }
                // if (states.contains(WidgetState.selected)) {
                //   return const BorderSide(color: Colors.brown, width: 2);
                // }

                return const BorderSide(color: Colors.amber, width: 2);
              },
            ),
          ),
        ),
      ),
      home: Scaffold(
        body: Center(
          child: Column(
            spacing: 20,
            mainAxisAlignment: MainAxisAlignment.center,
            children: <Widget>[
              SegmentedButton<Calendar>(
                segments: const <ButtonSegment<Calendar>>[
                  ButtonSegment<Calendar>(
                      value: Calendar.day, label: Text('Day'), icon: Icon(Icons.calendar_view_day)),
                  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},
              ),
              SegmentedButton<Calendar>(
                segments: const <ButtonSegment<Calendar>>[
                  ButtonSegment<Calendar>(
                      value: Calendar.day, label: Text('Day'), icon: Icon(Icons.calendar_view_day)),
                  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},
                onSelectionChanged: (Set<Calendar> newSelection) {
                  setState(() {
                    calendarView = newSelection.first;
                  });
                },
              ),
            ],
          ),
        ),
        floatingActionButton: FloatingActionButton(
          onPressed: () {},
          child: const Icon(Icons.add),
        ),
      ),
    );
  }
}

Before

Screen.Recording.2025-01-21.at.15.23.24.mov

After

Screen.Recording.2025-01-21.at.15.22.33.mov

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jan 21, 2025
@TahaTesser TahaTesser marked this pull request as ready for review January 21, 2025 16:01
@TahaTesser TahaTesser changed the title Fix SegmentedButton border doesn't respect hovered, focused state Fix SegmentedButton border doesn't respect hovered, focused, selected states Jan 22, 2025
@Piinks Piinks requested a review from QuncCccccc January 29, 2025 19:19
@victorsanni victorsanni self-requested a review January 29, 2025 19:19
@TahaTesser TahaTesser force-pushed the segmented_button_side_states branch from 97133ad to 5f4a194 Compare February 4, 2025 13:50
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

Based on the specs, it seems if a segment is disabled, we should be able to show the disabled state on border of the specific segment (not the whole SegmentedButton). So It's not clear what expected behavior would be when a specific segment is hovered, focused and pressed. Let me double check with the material team.

@QuncCccccc
Copy link
Contributor

Filed a bug and asked for clarification: b/394478041

@TahaTesser TahaTesser force-pushed the segmented_button_side_states branch from bbb0745 to 4ff767b Compare February 5, 2025 13:20
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

I haven't heard about the update from the material team, but I do see there's a token to indicate the outline of the whole segmented button, so I think this feature makes sense:)

Sorry for the late response. Trying my best to speed up! I'll make sure we make some progress this week!

@TahaTesser TahaTesser force-pushed the segmented_button_side_states branch from f0ce435 to 6825b07 Compare February 26, 2025 14:41
@TahaTesser TahaTesser removed the request for review from matanlurey February 26, 2025 14:42
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much for your patience.

@TahaTesser TahaTesser force-pushed the segmented_button_side_states branch from 6825b07 to 74e10d3 Compare March 12, 2025 10:59
@TahaTesser TahaTesser requested review from victorsanni and removed request for victorsanni March 12, 2025 11:00
@TahaTesser
Copy link
Member Author

@victorsanni Could you please approve? :)

Copy link
Contributor

@victorsanni victorsanni left a comment

Choose a reason for hiding this comment

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

LGTM

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 12, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 12, 2025
Copy link
Contributor

auto-submit bot commented Mar 12, 2025

autosubmit label was removed for flutter/flutter/161942, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@victorsanni
Copy link
Contributor

I was looking at the google testing failures, they are all image diffs. Seems like the disabled segmented button is now black instead of gray. I wasn't following the earlier conversation, but is that intended?

@QuncCccccc
Copy link
Contributor

QuncCccccc commented Mar 12, 2025

I'll look at the internal use case to see how they use the segmented buttons. I think we handle the disabled state correctly here🤔

@QuncCccccc QuncCccccc self-requested a review March 14, 2025 22:31
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
3 participants