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

AlertDialog.actions overrides ButtonTheme #38646

Closed
Levi-Lesches opened this issue Aug 15, 2019 · 16 comments
Closed

AlertDialog.actions overrides ButtonTheme #38646

Levi-Lesches opened this issue Aug 15, 2019 · 16 comments
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Comments

@Levi-Lesches
Copy link
Contributor

When making my app, I noticed the buttons in AlertDialog.actions looked a bit off -- they weren't conforming to MaterialApp.theme.buttonTheme.buttonColor. After checking the documentation, I found this under AlertDialog.build:

if (actions != null) {
  children.add(ButtonTheme.bar(
    child: ButtonBar(
      children: actions,
    ),
  ));
}

Why does AlertDialog provide its own ButtonTheme if it's not even going to give it an actual theme?

A temporary workaround I found was using the ambient ButtonTheme.colorScheme. So, if you want to make, say, a RaisedButton with the correct color, you could do:

final MaterialButton _button = RaisedButton(onPressed: () {});

// the build method

actions: [
  RaisedButton(
    child: Text ("OK"),
    onPressed: () {},
    color: Theme.of(context).buttonTheme.getFillColor(_button),
  )
]
// more code below

Output of flutter doctor -v:

[√] Flutter (Channel stable, v1.7.8+hotfix.4, on Microsoft Windows [Version
    10.0.18362.239], locale en-US)
    • Flutter version 1.7.8+hotfix.4 at C:\Users\levi\flutter
    • Framework revision 20e59316b8 (4 weeks ago), 2019-07-18 20:04:33 -0700
    • Engine revision fee001c93f
    • Dart version 2.4.0


[√] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
    • Android SDK at C:\Users\levi\AppData\Local\Android\sdk
    • Android NDK location not configured (optional; useful for native profiling
      support)
    • Platform android-28, build-tools 28.0.3
    • Java binary at: C:\Program Files\Android\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1343-b01)
    • All Android licenses accepted.

[√] Android Studio (version 3.4)
    • Android Studio at C:\Program Files\Android
    • Flutter plugin version 35.3.1
    • Dart plugin version 183.6270
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1343-b01)

[√] Connected device (1 available)
    • SM G950U1 • 988861314d554d4f52 • android-arm64 • Android 9 (API 28)

• No issues found!
@darrenaustin darrenaustin added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Aug 19, 2019
@Levi-Lesches
Copy link
Contributor Author

@darrenaustin I stumbled across your PR #37544. Does that fix this issue?

@darrenaustin
Copy link
Contributor

I think so. It changes the default look of the ButtonBar to use the primary color instead of the secondary, so it should match the Material spec now.

If you still want more control over the look of the buttons in AlertDialogs, you can now override their setting with the new ButtonBarTheme that you can set as part of your overall Theme that you set on your MaterialApp (see the last example in the PR description).

I'm going to close this issue. If this PR didn't resolve your issue, please reopen or file another issue. Thanks!

@Levi-Lesches
Copy link
Contributor Author

Right. What I was trying to focus on is that now you changed:

children.add(
  ButtonTheme.bar(
    child: ButtonBar(children: actions),	
  )
);

to:

children.add(
  ButtonBar(children: actions)
);

which should respect the ambient ButtonBarTheme defined in MaterialApp.theme.buttonBarTheme.

@Levi-Lesches
Copy link
Contributor Author

@darrenaustin when will your PR make it to Flutter? Do you know what version it will be?

@darrenaustin
Copy link
Contributor

@Levi-Lesches It went in commit: 9dce19e which is on master and entered the dev channel in v1.9.5. We don't have a specific timeline for when this will migrate to beta and then stable, but you can see the process at:

https://github.com/flutter/flutter/wiki/Flutter-build-release-channels

Which will also tell you how to switch to the dev channel if you just want to test it out to make sure it fixes your problem.

@Levi-Lesches
Copy link
Contributor Author

This didn't fix the problem. The AlertDialog still defaults to standard colors, which is weird since I have it working in another place... Any advice?

@Levi-Lesches
Copy link
Contributor Author

Never mind the first instance doesn't work. Can you re-open the issue?

@darrenaustin
Copy link
Contributor

I am a little lost as to exactly what you are referring to. Can you provide an runnable example showing how it isn't working for you? I just tried the following and am not seeing a problem with the buttons in an AlertDialog picking up the buttonTheme.buttonColor:

import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData.from(colorScheme: ColorScheme.light()).copyWith(
        buttonTheme: ButtonThemeData(buttonColor: Colors.green),
      ),
      darkTheme: ThemeData.from(colorScheme: ColorScheme.dark()),
      home: Builder(
        builder: (BuildContext context) {
          return Scaffold(
            appBar: AppBar(
              title: Text('Alert Dialog'),
            ),
            body: Center(
              child: Column(
                mainAxisAlignment: MainAxisAlignment.center,
                children: <Widget>[
                  RaisedButton(
                    child: Text('Alert me!'),
                    onPressed: () {
                      showDialog(
                        context: context,
                        builder: (BuildContext context) {
                          return AlertDialog(
                            title: Text('Alert! Alert!'),
                            content: SingleChildScrollView(
                              child: ListBody(
                                children: <Widget>[
                                  Text('Like something happened.'),
                                ],
                              ),
                            ),
                            actions: <Widget>[
                              RaisedButton(
                                child: Text('SURE'),
                                onPressed: () {
                                  Navigator.of(context).pop();
                                },
                              ),
                            ],
                          );
                        },
                      );
                    },
                  ),
                ],
              ),
            ),
          );
        },
      ),
    );
  }
}

@darrenaustin darrenaustin reopened this Aug 27, 2019
@darrenaustin darrenaustin added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 29, 2019
@Levi-Lesches
Copy link
Contributor Author

Levi-Lesches commented Aug 30, 2019

Sorry I still haven't had time to fully test this. I'm trying to rush a tight deadline, but I found something interesting here. I was making a companion app for my original app (an admin console which required access that normal users should not have/need), and while trying to make a dialog I noticed that the RaisedButton was the default color of blue (I hadn't overridden the theme yet), but so was the text! I had a FlatButton next to it and its text color was also that default blue, so I manually changed the RaisedButton's child text color to white, which solved the problem. Here is the code for that:

void showError(BuildContext context) => showDialog(
  context: context,
  builder: (_) => AlertDialog(
    title: Text ("Login failed"),
    content: Column(
      mainAxisSize: MainAxisSize.min,
      children: [
        Text (
          "Could not log into your account",
          textScaleFactor: 1.5,
        ),
        SizedBox(height: 10),
        Text(
          "A problem prevented you from logging in. "
          "Please contact someone from IT for help."
        )
      ],
    ),
    actions: [
      RaisedButton(
        child: Text (
          "Close",
          style: TextStyle(color: Colors.white),
        ),
        onPressed: Navigator.of(context).pop,
      )
    ]
  )
);

(Note that I later got rid of the FlatButton)
Without:

style: TextStyle(color: Colors.white),

the text and the button filling would both be blue.

Again, apologies for not having time to test this out, I had to just quickly fix it and move on. I might have some time this week to figure this out some more. I'm particularly interested in ButtonThemeData.getFillColor(MaterialButton), ButtonThemeData.getTextColor(MaterialButton), as well as AlertDialog.build as described in my original comment.

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 30, 2019
@Levi-Lesches
Copy link
Contributor Author

Ok so I properly started a new project yadda yadda. Basically this code:

import "package:flutter/material.dart";

const Color mySpecificBlue = Color(0xFF004B8D);

void main() => runApp(MainApp());

class MainApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(
    home: DialogPage(),
  );
}

class DialogPage extends StatelessWidget {
  @override
  Widget build(BuildContext context) => Scaffold(
    floatingActionButton: FloatingActionButton(
      child: Icon(Icons.info),
      onPressed: () => showDialog(
        context: context,
        builder: (_) => AlertDialog(
          title: Text ("This is a test"),
          actions: [
            RaisedButton(
              child: Text ("OK"),
              onPressed: () => Navigator.of(context).pop()
            )
          ]
        )
      )
    )
  );
}

yields this:
flutter_01
Which is new, since now I can't even see the text.

Giving the theme a new buttonColor and buttonThemeData.buttonColor (which one is preferrable? The docs should be a bit more explicit on this...) like so yields the same result:

class MainApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(
    home: DialogPage(),
    theme: ThemeData(
      buttonColor: mySpecificBlue,
      buttonTheme: ButtonThemeData(
        buttonColor: mySpecificBlue,
      )
    )
  );
}

The only thing that seemed to work was giving buttonTheme a ColorScheme:

class MainApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(
    home: DialogPage(),
    theme: ThemeData(
      buttonTheme: ButtonThemeData(
        colorScheme: ColorScheme.light(
          primary: mySpecificBlue,
          onPrimary: Colors.white,
        ),
      )
    )
  );
}

which gives this:
flutter_02

This reveals that the text color was indeed the default blue all along. Additionally, onPrimary was not used for the text color. To get the text color to change I had to provide a secondary to ColorScheme:

class MainApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(
    home: DialogPage(),
    theme: ThemeData(
      buttonTheme: ButtonThemeData(
        colorScheme: ColorScheme.light(
          primary: mySpecificBlue,
          secondary: Colors.white,
        ),
      )
    )
  );
}

Which fixes it:
flutter_03

This is also discussed at issue #22745, but nobody seems to be looking into it. If you look at my comments there one could think to set the text color to primary or secondary, but as @willlarche points out, Material design dictates careful consideration here.

@Levi-Lesches
Copy link
Contributor Author

So to summarize, the problems are:

  1. The button's text color defaults to the primary color, which is the same as the button's default fill color.
  2. Adding buttonColor does not change the color in any way, only ThemeData.buttonTheme.colorScheme.primary affects the color.
  3. Adding onPrimary does not change the text color. Only ColorScheme.secondary is used for text color.

@Levi-Lesches
Copy link
Contributor Author

Any update on this? I would say it's a pretty big issue seeing as none of this is documented right now.

@Levi-Lesches
Copy link
Contributor Author

Hey @darrenaustin, I found why your code works and mine doesnt:

theme: ThemeData.from(colorScheme: ColorScheme.light()).copyWith(
  buttonTheme: ButtonThemeData(buttonColor: Colors.green),
),
darkTheme: ThemeData.from(colorScheme: ColorScheme.dark()),

ColorSchemes are documented as still being a process and not everyone (especially those who already finalized their theming before ColorSchemes were published) knows to use them. That's essentially what my tests above established. That no matter what changes I made to the theme, only adding ThemeData.buttonTheme.colorScheme worked. Can this be fixed to be backwards-compatible with "normal" theming until ColorSchemes fully replace the parameters in the ThemeData constructor?

@Levi-Lesches
Copy link
Contributor Author

Any updates on this? It's a pretty big issue IMO.

@Levi-Lesches
Copy link
Contributor Author

Looks like this is being handled in #54776, which basically redoes the entire theming system to be more consistent, so I'll close it here (@HansMuller is that correct?).

@github-actions
Copy link

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 Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

No branches or pull requests

2 participants