Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

[New rule] Widgets to use Theme #745

Open
NickBalnaves opened this issue Mar 20, 2022 · 3 comments
Open

[New rule] Widgets to use Theme #745

NickBalnaves opened this issue Mar 20, 2022 · 3 comments
Assignees
Labels
area-rules type: enhancement New feature or request

Comments

@NickBalnaves
Copy link

Please describe what the rule should do:
Any text styling or colors used in widgets to come from a Theme so these values can be shared throughout an app.

What category of rule is this? (place an "X" next to just one item)
[ ] Warns about a potential error (problem)
[x] Suggests an alternate way of doing something (suggestion)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about (it will be better if you can provide both good and bad examples):
Good:

Container(
  color: Theme.of(context).colorScheme.secondary,
  child: Text(
    'Text with a background color',
    style: Theme.of(context).textTheme.headline6,
  ),
)

Bad:

Container(
  color: Colors.red,
  child: Text(
    'Text with a background color',
    style: TextStyle(color: Color(0xFF32DD35), fontSize: 14),
  ),
)

Are you willing to submit a pull request to implement this rule?
No

@incendial
Copy link
Member

incendial commented Mar 20, 2022

@NickBalnaves hi, do you have different examples of getting the theme? I mean, the rule should probably trigger on something like:

final theme = Theme.of(context);
...
Container(
  color: theme.colorScheme.secondary,
  child: Text(
    'Text with a background color',
    style: theme.textTheme.headline6,
  ),
)

but are there more ways to get the theme?

@incendial incendial added type: enhancement New feature or request area-rules labels Mar 20, 2022
@NickBalnaves
Copy link
Author

NickBalnaves commented Mar 20, 2022

Hi @incendial, here's another example which should cause a lint but seems that it might be more difficult to catch:

ExampleWidget(Theme.of(context));

class ExampleWidget extends StatelessWidget {
  const ExampleWidget(final this.firstTheme);
  final ThemeData firstTheme;

  @override
  Widget build(BuildContext context) {
    final secondTheme = Theme.of(context);
    ...
  }
}

It's possible to define a new class to use for a theme instead of the default material Theme, something like this:

class CustomTheme extends StatelessWidget {
  ...
  static CustomThemeData of(BuildContext context) {
    ...

so it might be good to also add an override option for specifying your own theme.

@incendial
Copy link
Member

Thanks, got it. Right now looks hard to cover all the edge cases, but we can probably start with the partial support.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-rules type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants