Refactor `ToggleButtons` (remove `RawMaterialButton`) #99493

TahaTesser · 2022-03-03T17:09:28Z

minimal code sample

import 'package:flutter/material.dart';

const List<Widget> icons = <Widget>[
  Icon(Icons.ac_unit),
  Icon(Icons.call),
  Icon(Icons.cake),
];

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      title: 'Material App',
      home: ToggleButtonsSample(),
    );
  }
}

class ToggleButtonsSample extends StatelessWidget {
  ToggleButtonsSample({Key? key}) : super(key: key);
  final ValueNotifier<int> _toggleValue = ValueNotifier<int>(0);

  @override
  Widget build(BuildContext context) {
    final ColorScheme colorScheme = Theme.of(context).colorScheme;

    return Scaffold(
      appBar: AppBar(
        title: const Text('ToggleButtons Sample'),
      ),
      body: Center(
        child: ValueListenableBuilder(
          valueListenable: _toggleValue,
          builder: (BuildContext context, int value, Widget? child) {
            return ToggleButtons(
              onPressed: (int newIndex) => _toggleValue.value = newIndex,
              borderRadius: const BorderRadius.all(Radius.circular(8)),
              selectedBorderColor: colorScheme.primary,
              selectedColor: colorScheme.primary,
              isSelected: <bool>[
                _toggleValue.value == 0,
                _toggleValue.value == 1,
                _toggleValue.value == 2,
              ],
              children: icons,
            );
          },
        ),
      ),
    );
  }
}

Description

Here this PR refactors ToggleButtons and use ButtonStyleButton to style ToggleButtons

highlight color not being translated to ElevatedButton (ButtonStyleButton sets highlight color to be transparent).

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

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

Piinks

NICE - all of the existing golden file tests pass! 🎉 (There are only two, but still, that's great)

Comparing this to the same refactor you are doing for FAB (#99753)

FAB is not becoming a ButtonStyleButton, but here it looks like ToggleButtons are.
FAB is not going to have a ButtonStyle style parameter like here, instead deferring to its numerated properties and inherited theme to create a ButtonStyle internally. ToggleButtons here allow for style to be set OR individual properties + theming.

I wonder if the FAB approach is cleaner, the style property here on ToggleButtons might not be necessary, but again I will defer to @HansMuller on general design since he wrote the model.

All of this work in FAB & Toggle is really great progress though. Thanks!

HansMuller · 2022-03-11T00:13:24Z

We should make sure that this PR also fixes #97302

HansMuller · 2022-03-11T00:14:49Z

I agree with Kate, this PR shouldn't include the extra step of adding a ButtonStyle style parameter to ToggleButtons; we should leave that for a separate proposal/PR.

TahaTesser · 2022-03-14T16:38:34Z

We should make sure that this PR also fixes #97302

This is now handled by TextButton widget in this refactor, can you please take a look

Mobile

Desktop (Web)

HansMuller · 2022-03-14T18:33:25Z

@TahaTesser - if this PR fixes #97302 it should say so in the PR's description and it should include a regression test (marked as a regression test for #97302 with a comment).

HansMuller · 2022-03-14T18:34:25Z

@TahaTesser - the screenshots in #99493 (comment) imply that the background color for the ToggleButtons isn't clipped to the ToggleButtons' shape?

TahaTesser · 2022-03-14T21:45:46Z

@HansMuller
Thanks so much for your time,

@TahaTesser - the screenshots in #99493 (comment) imply that the background color for the ToggleButtons isn't clipped to the ToggleButtons' shape?

This is just a parent container with color from #97302

ClipRRect still clipping the toggle buttons in this refactor
https://github.com/flutter/flutter/pull/99493/files#diff-153bdbb7a6739dd071bbb7b8230e3884dcb6cedbc21a78ef82c0e6a8166bb220R731

HansMuller · 2022-03-14T21:50:43Z

@TahaTesser - Good!

TahaTesser · 2022-03-14T22:49:06Z

@HansMuller
Added a regression test with a link to the issue

TahaTesser · 2022-03-15T09:57:49Z

@HansMuller
During refactoring noticed, there is no interactive example that showcases different configurations for ToggleButtons

Filed #100124, this will land after this PR (tester finders look for TextButton instead of RawMaterialButton).

HansMuller

This is looking good however I don't think the style parameter belongs in this PR; see #99493 (comment)

TahaTesser · 2022-03-17T15:48:26Z

This is looking good however I don't think the style parameter belongs in this PR; see #99493 (comment)

My apologies I missed removing that parameter, done.

packages/flutter/lib/src/material/toggle_buttons.dart

Piinks

@TahaTesser is this ready for another review? Or is it waiting on additional changes like the FAB one?

TahaTesser · 2022-03-23T10:07:09Z

@TahaTesser is this ready for another review? Or is it waiting on additional changes like the FAB one?

Hey @Piinks!
This needs another review. (my bad I thought I click request button before)

This Diagnosticable issue from FAB one doesn't affect this PR, all tests are passing

packages/flutter/lib/src/material/toggle_buttons.dart

HansMuller · 2022-03-23T16:28:12Z

@TahaTesser - I haven't tested or even compiled the code I've suggested for the _ForegroundColor and _BackgroundColor. If they're difficult let me know and I'll fix them.

TahaTesser · 2022-03-23T21:32:46Z

@TahaTesser - I haven't tested or even compiled the code I've suggested for the _ForegroundColor and _BackgroundColor. If they're difficult let me know and I'll fix them.

This is cleaner, thanks!
I made slight refinements and added these classes including the local ColorScheme variable as suggested and removed the redundant code due to these classes.

…ter/flutter#99493)

flutter-dashboard bot added f: material design framework labels Mar 3, 2022

TahaTesser requested review from HansMuller and Piinks Mar 3, 2022

TahaTesser force-pushed the refactor_toggle_buttons branch from 8058dd0 to 2f79f04 Compare Mar 4, 2022

Piinks reviewed Mar 10, 2022

View changes

TahaTesser force-pushed the refactor_toggle_buttons branch from 74a9652 to 4b1618d Compare Mar 11, 2022

TahaTesser force-pushed the refactor_toggle_buttons branch from 9d5b89e to 4284e86 Compare Mar 14, 2022

TahaTesser mentioned this pull request Mar 15, 2022

ToggleButtons: Add interactive example #100124

Open

8 tasks

HansMuller reviewed Mar 16, 2022

View changes

TahaTesser force-pushed the refactor_toggle_buttons branch from 4284e86 to 21d345d Compare Mar 17, 2022

HansMuller reviewed Mar 17, 2022

View changes

packages/flutter/lib/src/material/toggle_buttons.dart Show resolved Hide resolved

packages/flutter/lib/src/material/toggle_buttons.dart Show resolved Hide resolved

packages/flutter/lib/src/material/toggle_buttons.dart Show resolved Hide resolved

TahaTesser mentioned this pull request Mar 22, 2022

Refactor FAB (Remove RawMaterialButton) #99753

Open

8 tasks

Piinks reviewed Mar 22, 2022

View changes

TahaTesser requested a review from HansMuller Mar 23, 2022

HansMuller reviewed Mar 23, 2022

View changes

packages/flutter/lib/src/material/toggle_buttons.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/toggle_buttons.dart Outdated Show resolved Hide resolved

TahaTesser force-pushed the refactor_toggle_buttons branch from 213dfc0 to 9db5792 Compare Mar 23, 2022

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

5c6ad80

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 3a2929be315a (38 revisions) flutter/plugins#5139

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

9a9a613

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to fa4f90fdadfd (39 revisions) flutter/plugins#5140

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

996e148

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to f941ac4c8f14 (40 revisions) flutter/plugins#5141

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

02f9a49

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to c5819f4cb9ab (41 revisions) flutter/plugins#5142

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

6fa30db

…ter/flutter#99493)

TahaTesser deleted the refactor_toggle_buttons branch Apr 1, 2022

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to adcc07f9eb05 (42 revisions) flutter/plugins#5143

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

5098ffe

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 7971b412c16c (43 revisions) flutter/plugins#5144

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

e01f1c9

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 7971b412c16c (43 revisions) flutter/plugins#5146

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

1f4c41c

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to ff08bc4e8560 (44 revisions) flutter/plugins#5147

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

d4d4eba

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 56d7f90837c3 (45 revisions) flutter/plugins#5148

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

8ab102c

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 85b6e3bf0bd7 (46 revisions) flutter/plugins#5149

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

42629a7

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 3cab65b84884 (47 revisions) flutter/plugins#5152

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

71f0c48

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 510e2e78fcd2 (49 revisions) flutter/plugins#5153

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

9d2b5de

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 28505569463d (50 revisions) flutter/plugins#5154

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

782aaa8

…ter/flutter#99493)

engine-flutter-autoroll mentioned this pull request Apr 1, 2022

Roll Flutter from 66ed64be4fd8 to 723c88af5967 (51 revisions) flutter/plugins#5155

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this issue Apr 1, 2022

0b216f7a2 Refactor ToggleButtons (remove RawMaterialButton) (flut…

3648b7b

…ter/flutter#99493)

flutter / flutter Public

Refactor `ToggleButtons` (remove `RawMaterialButton`) #99493

Refactor `ToggleButtons` (remove `RawMaterialButton`) #99493

TahaTesser commented Mar 3, 2022 •

edited

Piinks left a comment

HansMuller commented Mar 11, 2022

HansMuller commented Mar 11, 2022

TahaTesser commented Mar 14, 2022

HansMuller commented Mar 14, 2022

HansMuller commented Mar 14, 2022

TahaTesser commented Mar 14, 2022 •

edited

HansMuller commented Mar 14, 2022

TahaTesser commented Mar 14, 2022 •

edited

TahaTesser commented Mar 15, 2022

HansMuller left a comment

TahaTesser commented Mar 17, 2022

Piinks left a comment

TahaTesser commented Mar 23, 2022 •

edited

HansMuller commented Mar 23, 2022

TahaTesser commented Mar 23, 2022 •

edited

flutter / flutter Public

Refactor ToggleButtons (remove RawMaterialButton) #99493

Refactor ToggleButtons (remove RawMaterialButton) #99493

Conversation

TahaTesser commented Mar 3, 2022 • edited

Description

Pre-launch Checklist

Piinks left a comment

HansMuller commented Mar 11, 2022

HansMuller commented Mar 11, 2022

TahaTesser commented Mar 14, 2022

Mobile

Desktop (Web)

HansMuller commented Mar 14, 2022

HansMuller commented Mar 14, 2022

TahaTesser commented Mar 14, 2022 • edited

HansMuller commented Mar 14, 2022

TahaTesser commented Mar 14, 2022 • edited

TahaTesser commented Mar 15, 2022

HansMuller left a comment

TahaTesser commented Mar 17, 2022

Piinks left a comment

TahaTesser commented Mar 23, 2022 • edited

HansMuller commented Mar 23, 2022

TahaTesser commented Mar 23, 2022 • edited

Refactor `ToggleButtons` (remove `RawMaterialButton`) #99493

Refactor `ToggleButtons` (remove `RawMaterialButton`) #99493

TahaTesser commented Mar 3, 2022 •

edited

TahaTesser commented Mar 14, 2022 •

edited

TahaTesser commented Mar 14, 2022 •

edited

TahaTesser commented Mar 23, 2022 •

edited

TahaTesser commented Mar 23, 2022 •

edited