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

ButtonThemeData does not define its colorScheme in the constructor #38655

Closed
Levi-Lesches opened this issue Aug 15, 2019 · 9 comments
Closed
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 initializing a new ButtonThemeData, the colorScheme property is not automatically generated, which pretty much makes all other parameters useless in some cases. For example, I'm trying to get the text color of a RaisedButton in an AlertDialog.actions button bar, and I need to override the theme (actually a really bad issue, IMO, see #38646). So I am trying to do:

actions: [
  RaisedButton(
    child: Text ("OK", style: TextStyle(
      // see issue #38646 for why this is needed 
      color: Theme.of(context).buttonTheme.getTextColor(RaisedButton(onPressed: () {}))
    )
  )
]

But I get an error:

Logs

I/flutter ( 8467): The following NoSuchMethodError was thrown building NotesBuilder(dirty, dependencies:
I/flutter ( 8467): [_LocalizationsScope-[GlobalKey#19ae2], _InheritedTheme]):
I/flutter ( 8467): The getter 'secondary' was called on null.
I/flutter ( 8467): Receiver: null
I/flutter ( 8467): Tried calling: secondary
I/flutter ( 8467):
I/flutter ( 8467): When the exception was thrown, this was the stack:
I/flutter ( 8467): #0      Object.noSuchMethod (dart:core-patch/object_patch.dart:50:5)
I/flutter ( 8467): #1      ButtonThemeData.getTextColor (package:flutter/src/material/button_theme.dart:590:28)
I/flutter ( 8467): #2      NotesBuilder.build (package:ramaz/pages/notes_builder.dart:34:43)
I/flutter ( 8467): #3      StatelessElement.build (package:flutter/src/widgets/framework.dart:3974:28)

I noticed this was because of ButtonTheme.getTextColor:

Color getTextColor(MaterialButton button) {
    if (!button.enabled)
      return getDisabledTextColor(button);

    if (button.textColor != null)
      return button.textColor;

    switch (getTextTheme(button)) {
      case ButtonTextTheme.normal:
        return getBrightness(button) == Brightness.dark ? Colors.white : Colors.black87;

      case ButtonTextTheme.accent:
        return colorScheme.secondary;

      case ButtonTextTheme.primary: {
        final Color fillColor = getFillColor(button);
        final bool fillIsDark = fillColor != null
          ? ThemeData.estimateBrightnessForColor(fillColor) == Brightness.dark
          : getBrightness(button) == Brightness.dark;
        if (fillIsDark)
          return Colors.white;
        if (button is FlatButton || button is OutlineButton)
          return colorScheme.primary;
        return Colors.black;
      }
    }

    assert(false);
    return null;
  }

This fails on ButtonTextTheme.accent, and since the error is that colorScheme was never defined, I went to the constructor:

 const ButtonThemeData({
    this.textTheme = ButtonTextTheme.normal,
    this.minWidth = 88.0,
    this.height = 36.0,
    EdgeInsetsGeometry padding,
    ShapeBorder shape,
    this.layoutBehavior = ButtonBarLayoutBehavior.padded,
    this.alignedDropdown = false,
    Color buttonColor,
    Color disabledColor,
    Color focusColor,
    Color hoverColor,
    Color highlightColor,
    Color splashColor,
    this.colorScheme,
    MaterialTapTargetSize materialTapTargetSize,
  }) : assert(textTheme != null),
       assert(minWidth != null && minWidth >= 0.0),
       assert(height != null && height >= 0.0),
       assert(alignedDropdown != null),
       assert(layoutBehavior != null),
       _buttonColor = buttonColor,
       _disabledColor = disabledColor,
       _focusColor = focusColor,
       _hoverColor = hoverColor,
       _highlightColor = highlightColor,
       _splashColor = splashColor,
       _padding = padding,
       _shape = shape,
       _materialTapTargetSize = materialTapTargetSize;

So colorScheme is never actually defined if not explicitly passed in. And because of #38646, passing in any of the other parameters doesn't actually matter for AlertDialog.actions.

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
@keshava19
Copy link
Contributor

Any update on the pull request ?

shihaohong pushed a commit that referenced this issue Oct 14, 2019
* Set default colorScheme for ButtonThemeData
@shihaohong
Copy link
Contributor

I'm closing the issue since #39627 fixes this

@shihaohong
Copy link
Contributor

I'm reopening this issue since we had to revert the original PR (#42854). The existence of both ThemeData and ButtonThemeData's colorScheme makes it impossible to define a default color scheme value for ButtonTheme, since doing so will never allow the buttons to ever access ThemeData's colorScheme

@shihaohong shihaohong reopened this Oct 17, 2019
@Levi-Lesches
Copy link
Contributor Author

Why is that property even there? Isn't ThemeData.colorScheme enough?

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this issue Nov 26, 2019
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this issue Nov 26, 2019
@HansMuller
Copy link
Contributor

@Levi-Lesches [sorry about the long (long) delay in responding. You're correct that it doesn't really make sense to have a ColorScheme just for the ButtonTheme. ButtonTheme has evolved in a way that has made it rather difficult to use. We're considering simplifying the way buttons are themed. Look for a concrete proposal early next year.

@Levi-Lesches
Copy link
Contributor Author

That's good to hear. Just a polite reminder that this issue is actively making #38646 much harder to work around.

@HansMuller
Copy link
Contributor

@Levi-Lesches As you may have already noticed, the long-promised proposal to remedy this problem, albeit by leaving it behind, has been published in #54776.

I'm going to close this issue in favor of #54776. If you have time, I'll hope you'll consider the proposal and add your feedback.

@Levi-Lesches
Copy link
Contributor Author

Checked it out, looks great! Can't wait for it to hit stable, the theming as of now really does get quite confusing.

@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

5 participants