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

Cannot change splash effect of ToggleButtons #119447

Closed
kaboc opened this issue Jan 29, 2023 · 5 comments
Closed

Cannot change splash effect of ToggleButtons #119447

kaboc opened this issue Jan 29, 2023 · 5 comments
Labels
r: solved Issue is closed as solved

Comments

@kaboc
Copy link
Contributor

kaboc commented Jan 29, 2023

I file this as a bug rather than a feature request as it seems like lack of a feature.

It is possible to change the appearance of the splash effect in various widgets (ElevatedButton, InkWell, etc), but even if I apply a different effect type to most widgets, ToggleButtons remains unchanged and therefore looks inconsistent. InkRipple.splashFactory is always used.

splashFactory: InkRipple.splashFactory,

Steps to Reproduce

  1. Execute flutter run on the code sample.
  2. Tap on TextButton.
  3. Tap on the toggle button to see the difference.

Expected results:

The sparkle effect configured in TextButtonThemeData is used in both TextButton and ToggleButtons.

Actual results:

The effect is only applied to the TextButton although ToggleButton uses TextButton internally.

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

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

class App extends StatefulWidget {
  const App();

  @override
  State<App> createState() => _AppState();
}

class _AppState extends State<App> {
  var _index = 0;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        textButtonTheme: TextButtonThemeData(
          style: TextButton.styleFrom(
            splashFactory: InkSparkle.splashFactory,
          ),
        ),
      ),
      home: Scaffold(
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              TextButton(
                child: const Text('TextButton'),
                onPressed: () {},
              ),
              const SizedBox(height: 8.0),
              ToggleButtons(
                isSelected: List.generate(2, (i) => i == _index),
                onPressed: (index) => setState(() => _index = index),
                children: [
                  for (var i = 0; i < 2; i++) Text('$i'),
                ],
              ),
            ],
          ),
        ),
      ),
    );
  }
}

ToggleButtonsThemeData has the splashColor argument, but does not have style nor splashFactory, and ToggleButton itself does not have relevant arguments either.

@exaby73 exaby73 added the in triage Presently being triaged by the triage team label Jan 30, 2023
@exaby73
Copy link
Member

exaby73 commented Jan 30, 2023

Hello @kaboc. I recommend you use SegmentedButton instead of ToggleButtons. SegmentedButton is meant to replace ToggleButtons and therefore it doesn't make sense to change and add to it's API now. Here is an example of the same code you provided, migrated to SegmentedButton:

import 'package:flutter/material.dart';

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

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

  @override
  State<App> createState() => _AppState();
}

class _AppState extends State<App> {
  var _index = 0;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        segmentedButtonTheme: const SegmentedButtonThemeData(
          style: ButtonStyle(
            splashFactory: InkSparkle.splashFactory,
          ),
        ),
        textButtonTheme: TextButtonThemeData(
          style: TextButton.styleFrom(
            splashFactory: InkSparkle.splashFactory,
          ),
        ),
      ),
      home: Scaffold(
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              TextButton(
                onPressed: () {},
                child: const Text('TextButton'),
              ),
              const SizedBox(height: 8.0),
              SegmentedButton<int>(
                selected: {_index},
                onSelectionChanged: (index) => setState(
                  () => _index = index.first,
                ),
                segments: [
                  for (var i = 0; i < 2; i++)
                    ButtonSegment(
                      value: i,
                      label: Text('$i'),
                    ),
                ],
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Due to the above reasons, I am closing the issue. Thank you

@exaby73 exaby73 closed this as not planned Won't fix, can't repro, duplicate, stale Jan 30, 2023
@exaby73 exaby73 added r: invalid Issue is closed as not valid and removed in triage Presently being triaged by the triage team labels Jan 30, 2023
@exaby73 exaby73 closed this as completed Jan 31, 2023
@exaby73 exaby73 added r: solved Issue is closed as solved and removed r: invalid Issue is closed as not valid labels Jan 31, 2023
@kaboc
Copy link
Contributor Author

kaboc commented Jan 31, 2023

@exaby73
Thank you for showing the example. I've now finished replacing the widgets, and I found SegmentedButton is not the same as ToggleButtons. It is not possible to apply a different border colour to only the selected button in SegmentedButton, which is possible in ToggleButtons.

SegmentedButton is meant to replace ToggleButtons and therefore it doesn't make sense to change and add to it's API now.

Does it mean widgets for Material 2 will be deprecated and then removed in favour of Material 3? It doesn't seem very nice that we all have to move on to Material 3 and make a compromise with reduced features like no border colour setting for a selected button as I mentioned above. I'm kind of tired of being affected by changes in Material Design every time it happens. Please clarify at least whether we can keep using Meterial 2 widgets.

@exaby73
Copy link
Member

exaby73 commented Jan 31, 2023

I can see that a border is not being applied. You ca raise an issue for it :) Ultimately, M2 will get deprecated in the coming months and years. This is inline with Material Design Specifications and best practices

@kaboc
Copy link
Contributor Author

kaboc commented Jan 31, 2023

Ultimately, M2 will get deprecated in the coming months and years.

Okay..., thanks you for clarification.

https://m3.material.io/components/segmented-buttons/overview#97f408eb-1e19-493f-919f-027b1ae9da00

It appears that selection is represented with a tick icon, not the border colour. I think it is the specification of the segmented button. It is very likely a new issue will be closed quickly even if I raise it, so I'd rather not do it. Thank you anyway.

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
r: solved Issue is closed as solved
Projects
None yet
Development

No branches or pull requests

2 participants