Refactor ToggleButtons (remove RawMaterialButton)

[TahaTesser]

fixes #99085
fixes #97302

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]

We should make sure that this PR also fixes #97302

[HansMuller]

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]

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]

@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]

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

[TahaTesser]

@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]

@TahaTesser - Good!

[TahaTesser]

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

[TahaTesser]

@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]

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.

[Piinks]

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

[TahaTesser]

@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

[HansMuller]

@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]

@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.

[math1man]

Hey, I think this change is a incorrect. #99493 (comment) does not look like the correct toggle button styling on Mobile.

[HansMuller]

@math1man - can you explain what aspect of the component doesn't look correct?

[math1man]

It's too tall. The standard height I believe is 32 dp, but it looks like 48 dp.

[HansMuller]

@TahaTesser - can you investigate? AFAICT we have not changed the geometry of ToggleButtons, however perhaps we've always had this wrong. I don't believe https://m3.material.io/ covers it (yet).

[Piinks]

The failure appears related to #97302, which this closed. Rather than allow for a height of 32, it seems like this enforced a height of 48 - IIRC that is the minimum required size for interactive elements. To reproduce this, the ToggleButtons.constraints were set with a min/max height of 32, but did not adhere to that.

[TahaTesser]

@HansMuller

I also found this in the Material Design components app, I will look into it and test it in an Android native app.

Preview

Perhaps there should Android equivalent of https://github.com/flutter/platform_tests/tree/master/ios_widget_catalog_compare

[TahaTesser]

@Piinks
Thank you for reverting

[HansMuller]

@TahaTesser - If a PR is created that relands the ToggleButton refactor, we'll need some additional regression tests to verify that the buttons handle input within 48 vertical without the button's outline occupying 48.