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

FloatingActionButtonTheme not set? #2

Closed
jlnrrg opened this issue Jan 15, 2021 · 4 comments
Closed

FloatingActionButtonTheme not set? #2

jlnrrg opened this issue Jan 15, 2021 · 4 comments

Comments

@jlnrrg
Copy link

jlnrrg commented Jan 15, 2021

Hey rydmike,
I noticed that a FAB created holds a weird color combination in relation to the upper flex_color_scheme (see attached picture)

FlexScheme definition
static FlexScheme usedFlexScheme = FlexScheme.mandyRed;

  static ThemeData lightTheme(BuildContext context) {
    final ThemeData baseTheme = FlexColorScheme.light(
      colors: FlexColor.schemes[usedFlexScheme].light,
      visualDensity: FlexColorScheme.comfortablePlatformDensity,
    ).toTheme;
    return baseTheme.copyWith(
        textTheme: GoogleFonts.firaSansTextTheme(baseTheme.textTheme));
  }

  static ThemeData darkTheme(BuildContext context) {
    final ThemeData baseTheme = FlexColorScheme.dark(
      colors: FlexColor.schemes[usedFlexScheme].dark,
      visualDensity: FlexColorScheme.comfortablePlatformDensity,
    ).toTheme;

    return baseTheme.copyWith(
        textTheme: GoogleFonts.firaSansTextTheme(baseTheme.textTheme));
  }

I also checked by directly setting the FAB (same result)

FAB definition
FloatingActionButton(
                  backgroundColor: Theme.of(context)
                      .floatingActionButtonTheme
                      .backgroundColor,
                  focusColor:
                      Theme.of(context).floatingActionButtonTheme.focusColor,
                  foregroundColor: Theme.of(context)
                      .floatingActionButtonTheme
                      .foregroundColor,
                  hoverColor:
                      Theme.of(context).floatingActionButtonTheme.hoverColor,
                  splashColor:
                      Theme.of(context).floatingActionButtonTheme.splashColor,
                  onPressed: fab?.onPressed,
                  child: fab?.icon,
                );

light mode
dark mode

In your documentation you have the code entry:

FloatingActionButtonThemeData(
  backgroundColor: colorScheme.secondary,
  foregroundColor: colorScheme.onSecondary),

where it not clearly shows, if I should set it myself, or if that is how the FloatingActionButtonTheme is set by the library.
Anyway, I assume that the FloatingActionButtonTheme is null, because ThemeData has no secondary attribute anymore.

@rydmike
Copy link
Owner

rydmike commented Jan 16, 2021

Hi @jlnrrg thanks for using FlexColorScheme and thanks for your question. Let's see if we can sort it out.

First a Question

For the above example, just to be clear, what color where you expecting on the FAB?

In the used example, with the used scheme FlexScheme.mandyRed it is shown as correctly using the used scheme's colorScheme.secondary color, as its background color. This is also the default and expected behavior of a FAB.

Did you expect some other color? If so, which one?

You can certainly modify it via custom theme modification too, more about that in an example further below.

Standard Theming Defaults

Some background basics regarding sub theme's in Flutter. Yes by default, with the new standard in Flutter, they are moving towards that all sub themes are NULL by default (background info) and also to color wise be defined by Theme.of(context).colorScheme. The actual widgets then implement their own default behavior when the widget's theme data is null or its color props are null.

For the FAB, this is in the FAB code defined so that when widget backgroundColor is null, then it uses FloatingActionButtonThemeData.backgroundColor of ThemeData.floatingActionButtonTheme. If that property is also null, then the ThemeData.colorScheme.secondary color is used.

Different look with Legacy Theme

By default the colorScheme in ThemeData will look a bit different if you create your standard Flutter theme with the
old theme: ThemeData() factory or use the newer theme: ThemeData.from() from a ColorScheme factory.

For the FAB case there is however not any practical difference. The first one in the past gave a FAB that used to use the so called accentColor , and it even depended on accentIconTheme to do so. Later this was removed, in favor of the above mentioned behavior where FAB, as default when all other props are null, uses ThemeData.colorScheme.secondary as its background color.

FlexColorScheme FAB Theme

You are right that FlexColorScheme.toTheme includes this definition for ThemeData.floatingActionButtonTheme

FloatingActionButtonThemeData(
  backgroundColor: colorScheme.secondary,
  foregroundColor: colorScheme.onSecondary),
)

In the latest stable Flutter version the above is actually not needed anymore, since it is the same color result as we would get if would just have left floatingActionButtonTheme to NULL in the ThemeData.

Why does it exist? It is a sad remnant, in some older versions of Flutter, a few stable versions back I think (FlexColorScheme existed for quite a while before I published it), I got some some deprecated warning if I did not add this definition to the ThemeData. However, and in part thanks to your question, I just noticed that it is no longer needed in latest stable version.

I now get the same color on the FAB even if I remove this definition from the FlexColorScheme.toTheme method (getter) and no deprecated warning from Flutter anymore (I just verfidied this). This means they have now removed the original reason for why I created this small sub-theme in the first place, and current stable now also defaults to the same theme that I defined, when it is not defined. So this config is no longer needed to produce the same targeted design.

I would actually prefer to not define it all anymore and keep it as null, but I cannot remove it anymore in version 1.x.x since it would be a small breaking change. If some user has made code that depends on ThemeData created with FlexColorScheme.toTheme having a property floatingActionButtonTheme that is not NULL and where the backgroundColor and foregroundColor in it are also not null, then if I remove this sub theme definition, while it would not change the produced theme, it would break those properties as they would become null potentially breaking someone's code. Yes, quite far fetched, but possible.

The upcoming null safe version will be a breaking change anyway (2.x.x), so I think I will remove this no longer needed definition from it. As I really only want to make sub-theme definitions when I really have to in order to produce the result I designed for FlexColorScheme.

None null Sub Themes in FlexColorScheme

You raised a good question here. Which sub-theme data objects are actually none NULL when using FlexColorScheme.toTheme?

Since there are indeed some, like the above mentioned FAB case for now. I should add a clear list to the documentation that lists these sub-themes and also which properties are set. I will see about getting that done for the next release as well. THanks for the input on this! 😃

It is there already in a way in the long list at the end, but it could be written in a much nicer cleaner way. The current list is just something I copied from my code comments.

ColorScheme's and FAB examples

I always like examples when I want to understand something, so I made one using your usage of the Mandy Red scheme as well. Maybe it will help explaining what is going on as well.

You can find the simple code for this example in this Gist.

If you make a default Flutter counter app, add FlexColorScheme package to it and replace the default counter app's main.dart with the gist you can follow along.

It contains three theme examples made from three different color schemes.

  1. The default Flutter light color scheme.
  2. The unmodified FlexColorScheme.toTheme based on FlexScheme.mandRed like you used too.
  3. A modified version of 2 that makes an identical theme as it, but changes the FAB to have theme data that uses the theme's colorScheme.primary color instead of colorScheme.secondary color for the FAB's sub-theme data. Just as an example of how you can make such a customized version of it.

The example is otherwise just the standard counter app, but I added the widget that I used in the package examples that shows key active scheme/theme colors. And I also show the actual color values of some key color attributes.

Example 1) Basic Flutter light default color scheme

If we then try the example with using the scheme for case 1:

(The Gist was saved with Example 1 as default example)

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    // Example 1)
    // The default Material Design Guide Theme from its color scheme.
    ThemeData fromMaterialScheme =
        ThemeData.from(colorScheme: ColorScheme.light());
    // Example 2)
    // A FlexColorScheme based theme from one of its built in color schemes.
    ThemeData fromFlexScheme = FlexColorScheme.light(
      colors: FlexColor.schemes[FlexScheme.mandyRed].light,
    ).toTheme;
    // Example 3)
    // Modified the above scheme and make some customization, keeping
    // all other properties as their where. In this case we change
    // the theme of the FAB to be primary color based.
    ThemeData myModifiedFlexScheme = fromFlexScheme.copyWith(
        floatingActionButtonTheme: fromFlexScheme.floatingActionButtonTheme
            .copyWith(
                backgroundColor: fromFlexScheme.colorScheme.primary,
                foregroundColor: fromFlexScheme.colorScheme.onPrimary));

    return MaterialApp(
      debugShowCheckedModeBanner: false,
      title: 'Flutter Demo',
      // Uncomment the theme you want to try.
      theme: fromMaterialScheme,  // Example 1
      // theme: fromFlexScheme, // Example 2
      // theme: myModifiedFlexScheme, // Example 3
      home: MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

We get the expected outcome looking like this:

image

The FAB theme data in Theme is NULL, but it using the theme.colorscheme.secondary color regardless based on its default behavior.

Example 2) FlexColorScheme based on mandyRed built-in scheme.

Let's just toggle the comments a bit to try example 2:

  // Uncomment the theme you want to try.
  // theme: fromMaterialScheme, // Example 1
  theme: fromFlexScheme, // Example 2
  // theme: myModifiedFlexScheme, // Example 3

Then we get this, it naturally has the same FAB color is in your example as well:

image

As excepted we can see that the secondary color is used, but it is also applied (unnecessarily in latest Flutter versions) via the floatingActionButtonTheme, that is no longer null, but it has the same color value as ´colorScheme.secondary`.

Example 3) Modify example 2) to use custom FAB theme.

In example 3 we modify the theme data from example 2, to make the FAB use a theme that is based on the active theme's colorScheme.primary color instead of the default colorScheme.primary. This is a common design that is sometimes preferred.

This is one way of making such a modification:

    // Example 2)
    // A FlexColorScheme based theme from one of its built in color schemes.
    ThemeData fromFlexScheme = FlexColorScheme.light(
      colors: FlexColor.schemes[FlexScheme.mandyRed].light,
    ).toTheme;
    // Example 3)
    // Modified the above scheme and make some customization, keeping
    // all other properties as their where. In this case we change
    // the theme of the FAB to be primary color based.
    ThemeData myModifiedFlexScheme = fromFlexScheme.copyWith(
        floatingActionButtonTheme: fromFlexScheme.floatingActionButtonTheme
            .copyWith(
                backgroundColor: fromFlexScheme.colorScheme.primary,
                foregroundColor: fromFlexScheme.colorScheme.onPrimary));

Let's use now that theme instead:

  // Uncomment the theme you want to try.
  // theme: fromMaterialScheme, // Example 1
  // theme: fromFlexScheme, // Example 2
  theme: myModifiedFlexScheme, // Example 3

As expected the result is then:

image

With a FAB that is primary colored, based on its new theme data.

Other topics

Built-in schemes in FlexColorScheme and their colors

Many of the built in scheme in FlexColorScheme use an approach where it is actually just using one color, using just slight toned variations of the same color for the different scheme colors. A few schemes, like eg the above used Mandy red scheme, has a two color approach, using different colors for primary and secondary and slightly offset saturations of those two base colors for the secondary colors of them.

Easier to use built-in schemes in FlexColorScheme

It was correctly pointed out to me that just setting up themes to use the built-in color schemes in FlexColorScheme is a bit verbose and cumbersome, with the FlexColorScheme.light or FlexColorScheme.dark factories. Referring then to a simple use case like this:

theme: FlexColorScheme.light( 
    colors: FlexColor.schemes[FlexScheme.mandyRed].light,
  ).toTheme;

darkTheme: FlexColorScheme.dark( 
    colors: FlexColor.schemes[FlexScheme.mandyRed].dark,
  ).toTheme;

I agree, it is cumbersome to to manually get the correct scheme this way via the map, if all you want to do is use an existing built-in scheme.

I was mainly focused on building custom scheme and using many of them in my own setups, so the colors property in the factories mainly serve that purpose. Extracting a defined FlexSchemeColor needed for the colors properties from the stored map with the enum key is unnecessarily verbose for just using the stored schemes.

In the very soon to be released version 1.4.0 there is a new scheme convenience property in the FlexColorScheme.light or FlexColorScheme.dark factories, to just use existing schemes with just their enum key value. In upcoming version1 .4.0 this will do the same as the above.

theme: FlexColorScheme.light(scheme: FlexScheme.mandyRed).toTheme;
darkTheme: FlexColorScheme.dark(scheme: FlexScheme.mandyRed).toTheme;

Old way is of course still supported too and is as before needed when you make custom schemes.


Finally

Not sure I any of my above ramblings helped or answered you question. I hope so, if not me know. 😃

BR
Mike

@rydmike
Copy link
Owner

rydmike commented Jan 16, 2021

DPT BTW

If you want to use the mandyRed scheme but for example just don't like the scheme's bluish secondary color, you can tell the FlexColorScheme.light factory to just use the 1st of its defined scheme colors, meaning just the primary color it has in its scheme. The rest of the theme's colors will then be computed as hues based on just this primary color.

If you in example 2 make this definition, set usedColors to 1:

    ThemeData fromFlexScheme = FlexColorScheme.light(
      colors: FlexColor.schemes[FlexScheme.mandyRed].light,
      usedColors: 1,
    ).toTheme;

You will instead get this result when you use it:

image

where secondary automatically becomes a slightly darker primary that is used by the FAB.


This property is documented in the API, but not actually shown in any of the examples. The live version of Flexfold demo app has a toggle where this property in a theme can be changed from 1...4 to use more or less of the pre-defined values in the color schemes to make the resulting theme based on the defined values, or compute missing ones.

@jlnrrg
Copy link
Author

jlnrrg commented Jan 16, 2021

Now I know that there is no length restriction on the issue comment field XD.
But for real, thank you for this more than elaborate answer.
As multiple points came to my awareness, I try to go through them in points and open new issues accordingly.

  1. Different look with Legacy Theme
    Personally I would enjoy a documentation entry for the correct usage of method, when creating a seperate ThemeData.
    As I understand you now, this would be the way to go:

    Usage of ThemeData.from
      Widget build(BuildContext context) {
        final ThemeData fromFlexScheme = ThemeData.from(
            colorScheme: FlexColorScheme.light(
          colors: FlexColor.schemes[FlexScheme.mandyRed].light,
        ).toScheme);
      }
    
  2. None null Sub Themes in FlexColorScheme
    Not what I had in mind, but a really good idea on your side to include a documentation on Null sub-themes.

  3. Built-in schemes in FlexColorScheme and their colors
    I think it is a great idea to include 2 color approaches beside the one color ones. On a few themes the differences between the chosen colors are too strong imo. So I created 2-color based scheme modification #3, to voice which theme would need a little adjustment from my point of view.

  4. higher resolution for overview of schemes
    I know that you have an overview of the different schemes on your website, with the scheme name hidden in each picture. But the overview picture on pub.dev has a too low resolution so the names can't be seen. I would encourage you to improve the documentation here for better access on choosing a theme. (see [Documentation] improvement of scheme overview #4)

  5. improve on github issue creation
    I am myself not sure how needed that is, but imo it would be great if you build github templates so one can predefine to open an issue or feature request. Otherwise there might be people hesitant to open a feature request, bc of the name issue.

  6. Final closing thought
    I now recon, that there is no error at all, but the chosen secondary color of manyRed just seemed out of place, Thus made me think that is was not set correctly

btw. I love using FlexColorScheme, and think it earns more exposure, so I hope you don't mind me linking to your project in my github profile.

@jlnrrg jlnrrg closed this as completed Jan 16, 2021
@rydmike
Copy link
Owner

rydmike commented Jan 18, 2021

@jlnrrg Regarding question 1 regarding how to make a separate ThemeData object, it is no different from assigning the object directly to the theme property.

While you can do as you show above in point/question 1, it will certainly give a ThemeData object, but with that approach you will get a ThemeData object that looses most of the theming features FlexColorScheme does with toTheme and only gets the colors.

You can certainly do so, if that is what you prefer, but it is not the recommended way. With the way you show above, FlexColorScheme returns its ColorScheme and you create a ThemeData with ThemeData.from using the returned ColorScheme() from FlexColorScheme.toScheme. This effectively strips out most of the ThemeData improvements FlexColorScheme does and then uses Flutter default SDK ThemeData.from to create the ThemeData() object.

This way is shown and used in example 5 as a way to demonstrate differences between default ThemeData.from and what FlexColorScheme.toTheme does. Read more here about why it is not the recommended way.

The correct FlexColorScheme way to do it is like this, just like I showed and used in the long example above:

  Widget build(BuildContext context) {
    ThemeData myThemeData = FlexColorScheme.light(
      colors: FlexColor.schemes[FlexScheme.mandyRed].light,
    ).toTheme;
  }

Now with release 1.4.0 you can simplify it like this:

  Widget build(BuildContext context) {
    ThemeData myThemeData = FlexColorScheme.light(scheme: FlexScheme.mandyRed).toTheme;
  }

It does the same thing, just a bit less verbose. The previous way still of course also also work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants