Add static `of` accessor methods to `ColorScheme` and `TextTheme`

[amal-stack]

trafficstars

The most common use case to lookup a ThemeData instance using Theme.of(context) is to access either the ColorScheme, the TextTheme, or both.

Before this change:

final colors = Theme.of(context).colorScheme;
final textTheme = Theme.of(context).textTheme;
final primaryTextTheme = Theme.of(context).primaryTextTheme;

or

final ThemeData(
      :colorScheme,
      :textTheme,
      :primaryTextTheme,
    ) = Theme.of(context);

After this change:

final colors = ColorScheme.of(context);
final textTheme = TextTheme.of(context);
final primaryTextTheme = TextTheme.primaryOf(context);

Primary Changes

This PR adds static of convenience methods to ColorScheme and TextTheme that delegate to the ThemeData's respective properties. The methods added are:

• ColorScheme.of(context) that returns Theme.of(context).colorScheme.
• TextTheme.of(context) that returns Theme.of(context).textTheme.
• TextTheme.primaryOf(context) that returns Theme.of(context).primaryTextTheme.

Side-effects

To allow the above changes to function, this PR adds:

• A theme.dart import to color_scheme.dart to access to Theme.
• A theme.dart import to text_theme.dart to access to Theme.
• A package:flutter/widgets.dart import to text_theme.dart to access BuildContext.
• The above import allowed getting rid of the same @docImport from text_theme.dart.

Documentation updates

This PR also updates the following documentation elements:

• Adds docs to the newly added members.
• Updates TextTheme's docs to instruct using TextTheme.of(context) instead of Theme.of(context).textTheme.
• Updates Theme.of to add a "See also" section to ColorScheme.of and TextTheme.of since these use cases are among the most common ones for Theme.of(context).

Fixes #72201.

Pre-launch Checklist

• [X] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [X] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [X] I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• [X] I signed the CLA.
• [X] I listed at least one issue that this PR fixes in the description above.
• [X] I updated/added relevant documentation (doc comments with ///).
• [X] I added new tests to check the change I am making, or this PR is test-exempt.
• [ ] I followed the breaking change policy and added Data Driven Fixes where supported.
• [X] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[navaronbracke]

This looks like a phenomenal change—pulling the color scheme from the theme data is something I do on a regular basis, and the framework does it a lot as well.

But I do have a couple of concerns.

Testing

This pull request needs simple tests—otherwise, in the future somebody could make a change that breaks these new methods and it might go unnoticed.

Tech Debt

At some point, we might want to convert Theme.of(context) so it uses an InheritedModel and can optimize its notifications more effectively. By adding these static methods today, we might incur a "technical debt" in the form of restricting our future options for API design. For instance, if we expand on this concept to cover every aspect of the theme data, it could be more practical to organize them all under the same namespace, like what we do for MediaQuery (i.e. Theme.colorSchemeOf, Theme.textOf, etc).

Overall, I believe the APIs introduced in this PR probably would be the ideal long-term solution… but it's probably best to leave a window to explore all the potential options, rather than landing this one quickly.

I do agree that converting the Theme to an InheritedModel is the way to go here. But since the Theme is so big in scope, I wonder if we can convert the Theme to become an InheritedModel, but introduce the aspects gradually, over multiple PR's? To give some context, the aspect class for MediaQuery is private, so perhaps we could do the following? (if at all possible)

1. Provide a private aspect class (similar to how MediaQuery works), with only the color scheme / text theme aspect for now
2. Make Theme an inherited model, that uses the private aspect class for the Theme
3. Use the new aspect for the ColorScheme.of(context) / TextTheme.of(context) implementation

One problem with this approach is "where do we define the Theme aspect class?", since its usages are probably in different files (i.e. color_scheme.dart). Perhaps using a regular class (so not prefixed with _ in the name) but not exporting it through the material library is a better option?

[amal-stack]

@nate-thegrate Thanks for the review. Regarding the concerns:

Testing

I have now added three tests that verify that the instance obtained via the new <Aspect>.of(context) is equivalent to obtaining them using Theme.of(context).<aspect>.

Tech Debt

Considering how large ThemeData is, I think switching to InheritedModel is a great idea.

By adding these static methods today, we might incur a "technical debt" in the form of restricting our future options for API design. For instance, if we expand on this concept to cover every aspect of the theme data, it could be more practical to organize them all under the same namespace, like what we do for MediaQuery (i.e. Theme.colorSchemeOf, Theme.textOf, etc).

It is worth noting that most component themes follow a common pattern in Flutter which is: For each component MyWidget, there is:

• A MyWidgetTheme: an inherited widget that extends InheritedTheme.
• A MyWidgetThemeData: a data class that contains the theme values for MyWidget.
• A ThemeData.myWidgetTheme property.

MyWidgetTheme also hosts a static of method that first looks if a MyWidgetTheme is in context, if not it just delegates to Theme.of(context).myWidgetTheme. So, all such component themes already have an of method that delegates to Theme.of (only when there is no ancestor widget of type MyWidgetTheme in the tree).

However, there are several instances that violate this pattern like AppBarTheme, BottomAppBarTheme, DialogTheme and TabBarTheme. These classes are just data classes (like ColorScheme) and not inherited widgets. All these classes have a static of method that merely and unconditionally delegates to: Theme.of(context).myWidgetTheme.

What I'm trying to highlight is even though if Theme were to switch to InheritedModel in the future, all these cases where component classes merely delegate to Theme.of(context).myWidgetTheme will have to be accounted for even if we were to organize the members under the same namespace. There are two ways to handle this:

1. Deprecate all the component theme methods MyWidgetTheme.of() (the ones that violate the pattern) and suggest using Theme.myWidgetThemeOf() instead. This will require users to change their code.
2. (No user changes required) Delegate all the component theme methods MyWidgetTheme.of() to Theme.myWidgetThemeOf() like:

static AppBarTheme of(BuildContext context) => Theme.appBarThemeOf(context);

If the first option is chosen, merging this PR will only eventually lead to all the newly added methods here being deprecated. In case of the second option, which seems more probable, these methods will only have to be changed from:

static ColorScheme of(BuildContext context) => Theme.of(context).colorScheme;

to

static ColorScheme of(BuildContext context) => Theme.colorSchemeOf(context);

Also:

final colorScheme = Theme.colorSchemeOf(context);

is a bit longer than:

final colorScheme = ColorScheme.of(context);

So, once Theme has been switched to InheritedModel, these methods could serve as a nice shorthand to use the inherited model and also prevent the users from still using Theme.of.

[nate-thegrate]

Perhaps using a regular class (so not prefixed with _ in the name) but not exporting it through the material library is a better option?

I don't think this has been done in the framework before, but I'm a fan! I'm also toying with the idea of giving public access to the class…

// this PR
final colors = ColorScheme.of(context);
return FilledButton(
  style: FilledButton.styleFrom(
    background: colors.tertiaryContainer,
    foreground: colors.onTertiaryContainer,
  ),
  // ...
);


// eventually:
return FilledButton(
  style: ButtonStyle.aspect(
    surface: const MaterialSurface.tertiaryContainer(),
  ),
  // ...
);

If we had a public API that essentially just contained instructions for how to parse & apply the theme data, it could potentially lead to significant performance gains, as dependency-based rebuilds are pushed into the leaf widgets. (I have a WIP design doc for this, though progress is on hold until I finish up some stuff in the animation library.)

[justinmc]

Do we have an open issue anywhere about the Theme as InheritedModel idea? There's a lot of good discussion here that hopefully we can build on.

[nate-thegrate]

Yes indeed!

https://github.com/flutter/flutter/issues/154567