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

[Proposal] Better support for ThemeExtensions #93

Open
bdlukaa opened this issue Sep 16, 2022 · 4 comments
Open

[Proposal] Better support for ThemeExtensions #93

bdlukaa opened this issue Sep 16, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@bdlukaa
Copy link
Contributor

bdlukaa commented Sep 16, 2022

We can take advantage of Material's theme extensions to replace the current mix theming. Currently, we define global variables and attach value to them in a MixThemeData, so that we can use globally.

Proposal

A callback based function theme<T extends ThemeExtension>((T)) that can be called inside a Mix. The mixer will look into every attribute for a theme and perform the operation based on the current context.

Something like:

final mix = Mix(
  bgColor(theme<MyColors>((myColors) => myColors.brandColor)),
  textColor(theme<MyColors>((myColors) => myColors.danger))),
);

// This is from the official example. Link attached below
@immutable
class MyColors extends ThemeExtension<MyColors> {
  const MyColors({
    required this.brandColor,
    required this.danger,
  });

  final Color? brandColor;
  final Color? danger;

  ...
}

Other things I have considered
Making theme more readable and usable with something like

final mix = Mix(
  bgColor(theme<MyColors>().brandColor),
  textColor(theme<MyColors>().danger),
);

But I can't seem to find a way where theme would have access to the current context

Additional context
https://api.flutter.dev/flutter/material/ThemeData/extensions.html

@leoafarias
Copy link
Member

@bdlukaa will review in detail. A few things to consider is that Mix should not be dependent on only Material design tokens, they also will need to be able to use them and/or create custom ones for their own design system.

@leoafarias
Copy link
Member

@bdlukaa I like this approach and could simplify a lot of our theming logic. I wonder if we can use this for spacing, sizes and so on. Could you do a POC for the Space theme data, and we can validate the implementation

@leoafarias leoafarias changed the title [proposal] Replace mix theming with material theme extensions [Proposal] Replace mix theming with material theme extensions Oct 9, 2022
@leoafarias
Copy link
Member

@bdlukaa while taking a closer look into this, I don't think ThemeExtensions replace the Mix Tokens, especially if you are trying to accomplish something like theme<MyColors>().danger, I am going to go through a bit and see if I can make any advance on the separation of Mix Tokens and the ThemeExtensions

@leoafarias leoafarias changed the title [Proposal] Replace mix theming with material theme extensions [Proposal] Better support for ThemeExtensions Jan 16, 2024
@leoafarias
Copy link
Member

We have rewritten large parts of Mix, which better supports a lot of Material Theme features. However, I have renamed this to highlight better support for ThemeExtensions.

@leoafarias leoafarias added the enhancement New feature or request label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants