rydmike / flex_seed_scheme

A more flexible version of Flutter's ColorScheme.fromSeed
Other
24 stars 2 forks source link

Why change FlexTones.vividSurfaces? My app is ugly now #17

Closed sgehrman closed 3 months ago

sgehrman commented 3 months ago

I picked vividSurfaces as my default for dark mode, now it's ugly. Why did this change?

Was: Screenshot-20240716221245-2558x1706

Now:

Screenshot-20240716223119-2462x1555

rydmike commented 3 months ago

If you are referring to and using the latest version of FSS 3.1.1 or even 3.1.0, it no longer sets the color ColorScheme.background to any value, so Flutter gives it the same value as ColorScheme.surface as fallback.

This is then again assigned to ThemeData.scaffoldBackgroundColor by default in Flutter's ThemeData factory.

The reason why it no longer sets a color for the background color in a ColorScheme is that Flutter 3.22 and later has deprecated the color, meaning it should no longer be used at all. If I use it, I get a penalty in the package score, for using and referencing a deprecated property, also later when Flutter totally removes the property, the app and the package would break.

What you can do to fix the look of your app (assuming it is using default color mappings) is:

1) Use FSS 3.0.0 that still does the no longer allowed color mapping. Works as a temp fix, but will eventually break and not compile, in a year or so.

Better option:

2) Fix the theme color mapping.

The in Flutter 3.22 and later released ColorScheme contains many new shades of surface colors that can be used instead of background. Use one of them as your ThemeData.scaffoldBackgroundColor.

In your ThemeData definition you could try:

ThemeData(
   scaffoldBackgroundColor: colorScheme.surfaceContainer,
);

Or any of the other of the new surface colors that have another shade than surface, like:

Depending on the level of difference you want in between the darker surface colors on the dials and their background. None of them will be an exact match for the past background color, but visually it should be similar enough.

This change in ColorScheme is unfortunate, but the reason why I had to do it, is in Flutter 3.22 and later.

On the flip side there is now 7 new different shades of surface colors, instead of the deprecated ones background and surfaceVariant. So a lot more options that can be used to tune the look, but you will have to select the color you want to use as the scaffoldBackgroundColor.

Another pending change in Flutter is that scaffoldBackgroundColor in ThemeData will also be deprecated and the property will be moved to a new sub/component theme, see issue: https://github.com/flutter/flutter/issues/91772. But that is an issue for another day :)

Edit:

Since I can't see any code showing how you had mapped colors to your theme and widgets, the above might not be the exact thing you need to change to make it match your past design, but it should give you the basic idea. Hope it helps, let me know if you need more guidance.

sgehrman commented 3 months ago

I'm not using background. Most of the surface colors in vividSurface are basically the same color now. Before it was better. I don't want my card color to be indistinguishable from the background like in the screenshot.

The code is new and I'm not doing anything weird as far as I know and not using any deprecated colors.

rydmike commented 3 months ago

Before there were only 3 different surface shades in FlexSeedScheme package and in the ColorScheme class itself:

Screenshot 2024-07-17 at 14 27 33

Now there are 8 different shades:

Screenshot 2024-07-17 at 14 27 51

Past background and surfaceVariant have both been deprecated in Flutter 3.22.

Even if you did not use these colors directly, Flutter still does by default and e.g. background then defaults to surface, since I can no longer assign it a color value without breaking rule of not using deprecated memebers.

Question?

Are you by any chance using FlexColorScheme and not FlexSeedScheme directly?

If you are using FlexColorScheme then yes most surface colors will be the same, because FlexColorScheme is not yet available for Flutter 3.22.

https://docs.flexcolorscheme.com/

Screenshot 2024-07-17 at 14 28 48

FlexColorScheme is is not yet producing a ColorScheme that is fully compatible with Flutter 3.22, it will not break, but most surface colors will indeed be the same color, as they are new and if not handled at all, they all default to surface in Flutter.

New FlexColorScheme 8.0.0 is still WIP. Flutter 3.22 changed/modified/added over 2500 lines of code dealing with the new ColorScheme and its defaults, so it is fair bit of things that must change in FlexColorScheme too. Some of the changes will also be breaking as maintaining backwards compatible is impossible due to all the things Flutter 3.22 broke and changed.

Flutter 3.22 is one of the biggest breaking changes I have seen when it comes to Material theming.

Before working FlexColorScheme I had to do major development on FlexSeedScheme that FlexColorScheme 8.0.0 will use, it will also need to do major changes to component theme defaults. Plus since FlexColorScheme can also do theming without using seed generated schemes, I needed to com up with a way to do that with all the new surface colors as well. Got all the pieces now. But it will still be some time before FlexColorScheme 8.0.0 is released, there will be test/dev releases of it first.

sgehrman commented 3 months ago

I'm using FlexSeedScheme directly and am on 3.22.

Here's the colors I'm getting:
Screenshot-20240717145338-397x657

surface is used for the scaffold background (background color to use according to docs)

and surfaceContainerLow is the Card color (default from flutter)

and they are both identical so now I don't have cards.

sgehrman commented 3 months ago

Also I must say I'm kind of disappointed in all these tones and schemes. On desktop at least I had to go out of my way to test them all to find ones that look half way decent on desktop. Maybe on a small mobile screen things look ok, but the difference between the background and the cards is not good on most of these tones.

rydmike commented 3 months ago

Thanks for this additional info. To replicate your setup and understand the problem I would really need a reproduction sample, but I think I got it and for your specific legacy need I have suggestion at the end.

But, yes in FlexTones.vividSurfaces the surface tone was modified to fit into the new ColorScheme, from 20 down to 10, perhaps I should have brought it down to its default 6, there is really very little wiggle room for its tone. I might have to do that, so it does not clash with tone for surfaceContainerLow that also uses 10, that Card uses by default now.

Vanilla surface is just not supposed to have a very high tone value anymore, so it had to be changed into its range, but I should have gone for 6, or maybe added tone 7 to make it more "vivid" than the default.

These are the colors I get for all the surface shades (but the result will depend on used primary seed too or custom surface seed, and I don't know yours):

Screenshot 2024-07-17 at 23 29 18

The surface colors are all distinguishable from each other, well on a color accurate monitor that don't crush blacks, many do and G's Material designers probably all use Mac that have decent color accuracy, but many desktop monitors don't, unless calibrated. But yes as mentioned above, surface and surfaceContainerLow use the same tone, and that is not good, I missed that, probably have to take it down to 6 for surface anyway, which is its default, or maybe introduce tone 7 and use it, but just sticking to 6 is the obvious and correct updated Material ColorScheme choice.

Putting tone 20 on surface in dark mode is not a good idea anymore with all the new default colors and color mappings in Flutter 3.22. Still if that is what your design was made with and it looks good for your app, you can put it back. FlexTones has a copyWith method, so you can easily change a few tones that don't fit your needs.

Try this in your dark scheme:

tones: FlexTones.vividSurfaces(Brightness.dark).copyWith(surfaceTone: 20),

That puts the old much lighter grey color back on surface color and thus also makes the dark mode background color much less dark, as before.

You can also make totally custom FlexTones mappings, there is an example in the readme. I fact all the built-in ones are just configs made in the same manner you can make your own custom versions. You can use them as examples or starting points for custom config ideas/needs, or as above just tweak one tone that does not fit your needs.

sgehrman commented 3 months ago

I just want it to look good. I fixed it on my end. But it would be great if all the variants looks great (where cards pop off the surface and look good)

rydmike commented 3 months ago

Thanks @sgehrman and I completely understand your frustration, can't begin to tell you how much work the new ColorScheme has and is still causing me.

And yes assigning colors from the different surface colors in ColorScheme to ThemeData colors like scaffoldBackgroundColor and relevant colors on component themes, eg Card, is the way you are suppose to use them to adjust the look of your app design to your goal.

That said, the below should as mentioned restore the previous tones style of FlexTones.vividSurfaces:

final ColorScheme colorSchemeLight = SeedColorScheme.fromSeeds(
    brightness: Brightness.light,
    primaryKey: mySeedColor,
    tones: FlexTones.vividSurfaces(Brightness.light).copyWith(surfaceTone: 95),
  );

final ColorScheme colorSchemeDark = SeedColorScheme.fromSeeds(
    brightness: Brightness.dark,
    primaryKey: mySeedColor,
    tones: FlexTones.vividSurfaces(Brightness.dark).copyWith(surfaceTone: 20),
  );

The Dark design of this style goes so much against to current design intent of the surface color, even the 10 I used as compromise does and I am going to have to Fix that and take it down to 6 as mentioned earlier.

The above will remain a working fix for the legacy style, I will add a note to the change log to use it for those that need it.

Thanks for highlighting the challenges with this migration.