ColorScheme Theme Quality Issues - Gaps in completeness and inconsistencies

[rydmike]

Issue description

ColorScheme based themes create ThemeData that is inconsistent with the provided ColorScheme. Some sub themes are also incomplete when using ColorScheme based themes because Theme.from(colorScheme) does not set all Theme() color properties used by sub themes.

For broader adoption of ColorScheme based theming, it would be useful if it behaved as might be expected and did not leave remnants of the default colors from Theme() factory in the resulting theme from a Theme.from(colorScheme) factory, where one would expect there should be colors from the colorScheme in use.

This issue demo uses ColorScheme.light() and ColorScheme.dark() and Theme.from(colorscheme) for each of them to demonstrate incompleteness and inconsistencies in the resulting light and dark themes and to show that many Theme() color properties do not get any colors from the used ColorScheme applied. This makes the resulting ThemeData inconsistent with the ColorScheme used to create the ThemeData. In some cases this also affects default colors on some widgets, making the situation potentially even more confusing.

Since the issue can easily be remedied with additional manual custom theming e.g. with copyWith, after creating a ColorScheme based theme with Theme.from(colorScheme), it is not a big issue, but the current situation does seem like an oversight and the incomplete and conflicting results may be confusing to new developers.

It would be useful and more elegant if when using ColorScheme and Theme.from(colorScheme) it created more complete matching ThemeData results, were none of its color properties were left with Theme() factory defaults that potentially conflict with the used ColorScheme and also that no widgets were left with default colors that do not match the ColorScheme used to create ThemeData from ColorScheme.

Steps to Reproduce

This issue applies to all current Flutter versions and all channels.

The source code for the issue demo can be found here:
https://gist.github.com/rydmike/b59cea071eefdbe7c1332b2d014bc156

And the issues can easily be demonstrated and tried with this DartPad:
https://www.dartpad.dev/b59cea071eefdbe7c1332b2d014bc156

Results and description of gaps and issues with ColorScheme based themes

toggleableActiveColor

ColorScheme based themes will get the default dark theme based toggleable color and not the secondary color from the dark
ColorScheme. The resulting color may or may not fit with the used colors in ColorScheme based theme. The default dark theme color is close to ColorScheme.dark() so it works fairly well for the default values, but not for general usage.

Proposed fix:
ThemeData.from() should also set:
toggleableActiveColor: colorScheme.secondary
for better backwards compatibility and so that the dark mode toggleableActiveColor also matches the ColorScheme based theme in dark mode and not only light mode.

---

primaryColorDark

ColorScheme based themes do not set any color at all for the Theme() color primaryColorDark. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set primaryColorDark to a slightly darker hue based on colorScheme.primary for light themes.

---

primaryColorLight

ColorScheme based themes do not set any color at all for the Theme() color primaryColorLight. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set primaryColorLight to a lighter hue based on colorScheme.primary for light themes.

---

secondaryHeaderColor

ColorScheme based themes do not set any color at all for the Theme() color secondaryHeaderColor. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set secondaryHeaderColor to a much lighter hue based on colorScheme.primary for light themes.

---

Text selection when using ColorScheme based themes

The new "TextSelectionThemeData" and "useTextSelectionTheme" address the issues below, but only when used in TextField() and SelectableText(). Still the color properties in the resulting ThemeData are left at their default values, which may be confusing and they also do not match the applied ColorScheme based theme.

The useTextSelectionTheme cannot be set with ColorScheme directly, an extra copyWith (inconvenient) is required and even then the ThemeData result values are left at their Theme() defaults, this may be confusing as they do not match the applied ColorScheme colors.

---

textSelectionColor

ColorScheme based themes do not set any color at all for the Theme() color textSelectionColor. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set textSelectionColor to a lighter hue based on colorScheme.primary for light themes and to colorScheme.secondary for dark themes.

Alternatively:
ColorScheme based themes could set the Theme() property to the colorScheme.primary based colors introduced with TextSelectionThemeData, this would not break older Theme() based themes and would make the property correctly ColorScheme based for themes based on ColorScheme.

---

textSelectionHandleColor

ColorScheme based themes do not set any color at all for the Theme() color textSelectionHandleColor. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set textSelectionHandleColor to a lighter hue based on colorScheme.primary for light themes and to a darker hue of colorScheme.secondary for dark themes.

Alternatively:
ColorScheme based themes could set the Theme() property to the colorScheme.primary based colors introduced with TextSelectionThemeData, this would not break older Theme() based themes and would make the property correctly ColorScheme based for themes based on ColorScheme and it would match the TextSelectionThemeData defaults.

---

cursorColor

ColorScheme based themes do not set any color at all for the Theme() color cursorColor. It only gets default value from default Theme factory for both light and dark mode. The resulting color may or may not fit with the used colors in ColorScheme based theme.

Proposed fix:
For a better fit with legacy themes and to match the colors in the ColorScheme the ThemeData.from() should set cursorColor to colorScheme.primary or some hue of it.

Alternatively:
ColorScheme based themes could set the Theme() property to the colorScheme.primary based colors introduced with TextSelectionThemeData, this would not break older Theme() based themes and would make the property correctly ColorScheme based for themes based on ColorScheme and it would match the TextSelectionThemeData defaults.

Note:
It is worth noticing that default Theme() uses the same fixed blue color for light and dark mode and that this blue color is not any variant of the default blue colors used in the default blue primary swatch, which is a bit odd. It is very close to the default blue hues in default Theme() and thus works well with the the default Theme(), but it does not work well with ColorScheme themes or any other none default and none blue based theme.

Flutter doctor

flutter doctor -v
[√] Flutter (Channel master, 1.22.0-10.0.pre.161, on Microsoft Windows [Version 10.0.18363.1016], locale en-US)
    • Flutter version 1.22.0-10.0.pre.161 at C:\Users\mryds\fvm\versions\master
    • Framework revision cba91c97d0 (20 hours ago), 2020-09-13 18:57:03 -0400
    • Engine revision 2fc054147e
    • Dart version 2.10.0 (build 2.10.0-121.0.dev)

[√] Android toolchain - develop for Android devices (Android SDK version 29.0.1)
    • Android SDK at C:\Users\mryds\AppData\Local\Android\sdk
    • Platform android-29, build-tools 29.0.1
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2019 16.6.5)
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
    • Visual Studio Community 2019 version 16.6.30320.27
    • Windows 10 SDK version 10.0.18362.0

[√] Android Studio (version 4.0)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin version 47.1.2
    • Dart plugin version 193.7361
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)

[√] IntelliJ IDEA Community Edition (version 2019.2)
    • IntelliJ at C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2018.3.1
    • Flutter plugin version 35.3.3
    • Dart plugin version 192.7402

[√] VS Code (version 1.49.0)
    • VS Code at C:\Users\mryds\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.14.1

[√] Connected device (4 available)
    • Windows (desktop) • windows    • windows-x64    • Microsoft Windows [Version 10.0.18363.1016]
    • Web Server (web)  • web-server • web-javascript • Flutter Tools
    • Chrome (web)      • chrome     • web-javascript • Google Chrome 85.0.4183.102
    • Edge (web)        • edge       • web-javascript • Microsoft Edge 85.0.564.44

• No issues found!

[pedromassangocode]

Hi @rydmike
Can you please provide a minimal reproducible sample code? Your code does seems to reproduce but I got lost on what is the real issue here while reading the issue description. Can you also provide the expected vs actual results?

Thank you

[rydmike]

Hi Pedro @pedromassango, sure will do, and you are right the presentation of the issue could be clearer and the sample could explain it better as well. I will update both to do so. I'll drop you another tag when done, might not be until tomorrow.

Ultimately parts of this quality issue/topic is also a bit of a matter of taste/opinion, but since the ColorScheme based theming is supposed to make creation of Material themes easier in Flutter, it would be nice if it did so in a more complete way when it comes to all properties in the resulting ThemeData. The way it is now there are too many remnants from the defaults Theme() factory.

This leads to that when you make themes with Theme.from(colorScheme) you end up with a lot of color properties in the resulting ThemeData that has nothing to do with the ColorScheme you defined, some of which even affect defaults for some widgets. At last I think this is bit inconsistent and could be confusing to newcomers. The ColorScheme and Theme.from(colorScheme) are a later addition to Flutter, than the Theme() factory that grew way too big. They are a nice addition, but they behave a bit like the afterthought they are, thus reducing their usefulness a bit.

Personally I like the ColorScheme based theming, but due to the gaps it has, we ended up just making our own variant of it that addresses the inconsistencies and gaps, my thinking around this issue is just that it would be nice if you would not have to do that.

[rydmike]

@pedromassangocode I updated the issue description to show the actual result and the expected result for both dark and light theme modes for the found ColorScheme resulting ThemeData gaps and inconsistencies.

The sample code was also updated to state actual Result and Expected result and if it is OK or FAILED for both light and dark theme modes.

The updated sample has the same URL as before and can be run in DartPad here:
https://www.dartpad.dev/b59cea071eefdbe7c1332b2d014bc156
If reproduced on current stable, beta, dev and master channels, the results are the same.

---

If the plan is to fully deprecate the direct ThemeData properties:

• toggleableActiveColor
• primaryColorDark
• primaryColorLight
• secondaryHeaderColor
• textSelectionColor
• textSelectionHandleColor
• cursorColor

And eventually totally remove them from ThemeData(), then the gaps are perhaps more understandable, but even so, while waiting for that, they could and should imo have useful and matching color values when creating themes from ColorSchemes.

[pedromassangocode]

Reproducible with latest master and other channels. Will label on the last three versions.

flutter doctor -v

[✓] Flutter (Channel master, 1.22.0-10.0.pre.200, on Mac OS X 10.15.6 19G2021, locale en)
    • Flutter version 1.22.0-10.0.pre.200 at /Users/pedromassango/dev/SDKs/flutter_master
    • Framework revision 25b8c095cd (71 minutes ago), 2020-09-16 01:27:03 -0400
    • Engine revision 1ef10b240e
    • Dart version 2.10.0 (build 2.10.0-124.0.dev)

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.1)
    • Android SDK at /Users/pedromassango/Library/Android/sdk
    • Platform android-30, build-tools 30.0.1
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.7)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.7, Build version 11E801a
    • CocoaPods version 1.9.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio (version 4.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    ✗ Dart plugin not installed; this adds Dart specific functionality.
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)

[✓] IntelliJ IDEA Community Edition (version 2020.2.1)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin version 49.0.4
    • Dart plugin version 202.7206

[✓] VS Code (version 1.48.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.14.1

[✓] Connected device (3 available)
    • macOS (desktop)  • macos      • darwin-x64     • Mac OS X 10.15.6 19G2021
    • Web Server (web) • web-server • web-javascript • Flutter Tools
    • Chrome (web)     • chrome     • web-javascript • Google Chrome 85.0.4183.102

! Doctor found issues in 1 category.

[HansMuller]

Thanks for the comprehensive report. As you may know, we're in the process of updating the theme system per https://flutter.dev/go/material-theme-system-updates.

@darrenaustin - can you provide an update about where we are with flutter.dev/go/text-selection-theme?

[darrenaustin]

Yes, thanks for the report. As Hans mentioned we are working to transition everything over to ColorScheme based colors. We obviously have a ways to go. Currently we are in the process of migrating the text selection colors, which is described in the design doc.

The first step landed with: #62014. This introduced the new TextSelectionTheme with an opt-in property in the ThemeData. The next step will land with #65044, which will change the default behavior to use this new selection theme instead of the top level properties in ThemeData. A final follow on PR will remove the opt-in flag and deprecate the top level properties (textSelectionColor, textSelectionHandleColor, and cursorColor).

After this transition all of these colors will be based off of colors from the ColorScheme unless the user overrides them.

[rydmike]

Thanks @HansMuller for the link to the design doc, and yes I do recall reading it before, just had another read, lgtm. 👍
I think it contains some new info from my previous read, eg on the AppBar topic and its brightness challenges, a bit related to this issue and discussion #50606. Pleased to see it is being considered as well.

@darrenaustin The improvements will certainly be welcome, looking forward to them! 😃

---

The reason why I actually brought this up was to shed a bit light on that the current situation can be a bit confusing to newcomers if they try to use ColorScheme in its current state of transitional flux. Additionally for seasoned users, if you try to migrate an app that used the traditional Theme() factory to setup its theme(s), to ColorScheme based ones, you are in for a few surprises if your app depends on and uses any of these ThemeData properties (and maybe some other too that I missed):

• toggleableActiveColor
• primaryColorDark
• primaryColorLight
• secondaryHeaderColor
• textSelectionColor
• textSelectionHandleColor
• cursorColor

Maybe the most obvious issue users will run into is the toggleableActiveColor in dark themes if you use ColorSchemes and Theme.from(colorScheme), since it is left at the Theme() default teal color and it actually impacts a few Widgets color in very obvious ways if you don't happen to use the default dark theme.

The properties primaryColorDark, primaryColorLight, secondaryHeaderColor are probably generally less used and not such a common issue for Widgets. However, if you have used them as a part of your apps design colors, then the gaps are more glaring when you try to migrate. I noticed it because we had references to them in an app I migrated to use ColorScheme based themes.

I did notice, and also mentioned in the above report, that the new TextSelectionTheme with the opt-in property in ThemeData nicely results in colors from the ColorScheme in the ThemeData (which btw are nicer and better choices than the default ones from the Theme() factory) in the TextField and SelectableText Widgets.

One small inconvenience issue with it is that when you create the ThemeData with Theme.from(colorScheme), you have to do an extra .copyWith(useTextSelectionTheme: true) on the resulting ThemeData in order to opt-in on it, you cannot opt-in when you make a theme from a color scheme, but there are a lot of other useful/needed ThemeData properties you might want to tweak anyway, so you will probably need to adjust the resulting ThemeData the with a copyWith anyway.

The opt-in case on TextSelectionTheme is included as a toggle in the issue demo sample as well. It can be observed in the demo that when used, yes indeed the ColorScheme colors you have in your ThemeData will be used by TextField and SelectableText so that works OK for new apps and their themes. BUT from a legacy compatibility situation, you are still left with values on the properties textSelectionColor, textSelectionHandleColor, cursorColor that do not match the ColorScheme you created your theme from. Thus if your app references these ThemeData properties they will still get the default values from the Theme() factory, that do not match your ColorScheme.

These are as I mentioned from the start all solvable minor issues that can dealt with, but they might be a bit confusing to someones who has not spent a lot of time getting to know the large and a bit messy ThemeData class and related classes.

Totally understand that you are working on cleaning it up and looking forward to the results.

---

We solved our ColorScheme backwards compatibility issues for now by making our own version of ColorScheme, well actually own version of the Theme.from(colorScheme) factory, that also sets the above ThemeData properties from colors in the ColorScheme in a fashion that addresses the gaps listed in this report, some values are computed based on values in the ColorScheme, since there are no appropriate values for it in the ColorScheme. This concerns primaryColorDark, primaryColorLight, secondaryHeaderColor.

I was just thinking that perhaps there is something that could be done within the framework to ease and support the migration towards using ColorScheme based theming as well?

---

Eventually, if/when the '21 grab-bag properties' mentioned in the "Flutter Material Theme System Update" document are fully deprecated and removed from ThemeData, we will have to make a bit deeper updates in apps. Not a big issue and relatively easy to do.

Regarding such a transition and eventually removing these properties, it is perhaps good to recognize that there probably are a lot of apps out there that uses custom/composed Widgets that just depend on properties from ThemeData for some of their styling. It is a common practical and pragmatic way to do things when you start with Flutter.

We no longer do this for obvious reasons. For more robust and really re-usable custom Widgets we create our own themes, which we based on how the newer sub-themes in ThemeData were made for Flutter SDK Widgets. Obviously this only makes sense for more elaborate custom Widgets that might be used as internal packages or even published on pub.dev.