Add support for other theme extensions in class fields #10

jakubtiteo · 2022-06-13T09:16:04Z

Example:

This three themes:

@tailor
class $_SomeTheme {
  static List<Color> appBackground = [AppColors.white, AppColors.black];
  @themeExtension
  static List<AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];
  static List<OtherThemeExtension> otherThemeExtension = [OtherThemeExtension(), OtherThemeExtension()];
}

@tailorComponent
class $_AnotherThemePart {
  static List<Color> navBarBackground = [AppColors.white, AppColors.black];
}

class OtherThemeExtension extends ThemeExtension<OtherThemeExtension> {
  OtherThemeExtension({
    this.floatingActionButtonColor = Colors.white,
  });

  final Color floatingActionButtonColor;
  
  // ...
}

Will result in following lerp method

  @override
  SomeTheme lerp(ThemeExtension<SomeTheme>? other, double t) {
    if (other is! SomeTheme) return this;
    return SomeTheme(
      appBackground: Color.lerp(appBackground, other.appBackground, t)!,
      anotherThemePart: anotherThemePart.lerp(other.anotherThemePart, t),
      otherThemeExtension:
          otherThemeExtension.lerp(other.otherThemeExtension, t),
    );
  }

This PR also adds generated lists of themes, example:

@tailor
class $_SomeTheme {
  @themeExtension
  static List<AnotherThemePart> anotherThemePartGeneratedConstructor = AnotherThemePart.themes;
}

@tailorComponent
class $_AnotherThemePart {
  static List<Color> navBarBackground = [AppColors.white, AppColors.black];
}

Generated code:

class AnotherThemePart extends ThemeExtension<AnotherThemePart> {
  static final themes = [
    dark,
    light,
  ];
}

# Conflicts: # packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart # packages/theme_tailor/lib/src/template/theme_class_template.dart

Rongix · 2022-06-13T09:44:22Z

I thought we could support nested themes like this:

@tailor
class $_AppTheme {
  List<CustomAppBarTheme> appBarTheme = [
    CustomAppBarTheme.otherLight,
    CustomAppBarTheme.otherDark
  ];
}

@Tailor(themes: ['otherLight', 'otherDark'])
class $_CustomAppBarTheme {
  List<Color> surface = [Colors.blue, Colors.deepPurple];
}

If one of the InterfaceType is ThemeExtension then we can simply use its copyWith and lerp.
WDYT?

Also ideally if this is a part of another theme extension class we would like to suppress it's generated extensions
so we might add smth

so maybe smth like that:

class TailorComponent extends Tailor {
  const TailorComponent({
    List<String> themes = const ['light', 'dark'],
    List<ThemeEncoder> encoders = const [],
  }) : super(themes: themes, encoders: encoders, isPart: true, themeGetter: ThemeGetter.none);
}

@TailorComponent()
class $_AnotherTheme {
  static List<Color> navBarBackground = [AppColors.white, AppColors.black];
}

jakubtiteo · 2022-06-13T10:24:36Z

Ok, I will implement generating code that uses copyWith and lerp from classes that implements ThemeExtension.
Maybe we can keep automatic constructor generation by using explicitly empty list, or something like this?

@tailor
class $_AppTheme {
  @inheritTailorTheme
  List<CustomAppBarTheme> appBarTheme = [];
}

@tailorComponent
class $_CustomAppBarTheme {
  List<Color> surface = [Colors.blue, Colors.deepPurple];
}

Rongix · 2022-06-13T10:50:05Z

Hm I think it would be preferable to generate list of available themes and supply it instead of the empty list. By default empty list would mean that no theme will be generated

  List<CustomAppBarTheme> appBarTheme = CustomAppBarTheme.themes;

(I'm not sure about the annotation on the "appBarTheme" - maybe we could name it just @component)

…t annotation

…itial value

# Conflicts: # packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

Rongix

@TailorComponent -> Just Tailor() but won't ever generate any extensions, it is meant for using in another @Tailor or if someone does not want extensions

~~@Component~~ / @themeExtension -> Annotation only for field. Hints generator that the type conforms to ThemeExtension and can call lerp() on it.
(When checking for annotation type, can use "const themeExtensionChecker = TypeChecker.fromRuntime(Extension);"

/// (When exporting only show themeExtension to not conflict with Flutter)
const themeExtension = ThemeExtension();

class ThemeExtension {
  const ThemeExtension();
}

Generated copyWith -> Not sure about this one, I would keep it simple for now and not worry about handling deep copy

We can't use smth like: "$AnotherThemePart" or "$AnotherThemePart" because this type might be private somewhere also it does not look super clean

Rongix · 2022-06-14T05:23:48Z

packages/theme_tailor/example/lib/theme_extension_in_field.dart

+@tailor
+class $_SomeTheme {
+  static List<Color> appBackground = [AppColors.white, AppColors.black];
+  static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];


Suggested change

static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];

static List<AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];

It is because we can't determine the type before it is generated?
Once more I suggest adding annotation directly on top of the field:

Rongix · 2022-06-14T05:40:27Z

packages/theme_tailor/lib/src/template/theme_class_template.dart

+      if (value.isThemeExtension) {
+        classParams.write('$key: this.$key.copyWith(');
+        for (final extensionField in value.themeExtensionFields) {
+          classParams.write('$extensionField: $key?.$extensionField,');
+        }
+        classParams.write('),');
+      } else {
+        classParams.write('$key: $key ?? this.$key,');
+      }


It looks like it currently copies whole object anyways, therefore can be treated as any other field

Rongix · 2022-06-14T05:53:06Z

packages/theme_tailor/lib/src/template/theme_class_template.dart

+      if (value.isThemeExtension) {
+        classParams.write('$key: this.$key.copyWith(');
+        for (final extensionField in value.themeExtensionFields) {
+          classParams.write('$extensionField: $key?.$extensionField,');
+        }
+        classParams.write('),');
+      } else {
+        classParams.write('$key: $key ?? this.$key,');
+      }


Suggested change

if (value.isThemeExtension) {

classParams.write('$key: this.$key.copyWith(');

for (final extensionField in value.themeExtensionFields) {

classParams.write('$extensionField: $key?.$extensionField,');

}

classParams.write('),');

} else {

classParams.write('$key: $key ?? this.$key,');

}

classParams.write('$key: $key ?? this.$key,');

Rongix · 2022-06-14T06:00:20Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+class _FieldListingVisitor extends SimpleElementVisitor<Object> {
+  final List<String> fieldNames = [];
+
+  @override
+  void visitFieldElement(FieldElement element) {
+    fieldNames.add(element.name);
+  }
+}


If we change to the same copyWith as other fields we don't need it

Suggested change

class _FieldListingVisitor extends SimpleElementVisitor<Object> {

final List<String> fieldNames = [];

@override

void visitFieldElement(FieldElement element) {

fieldNames.add(element.name);

}

}

Rongix · 2022-06-14T06:03:17Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+        isAnotherTailorTheme: hasTailorComponentAnnotation,
+        isThemeExtension: isThemeExtension,
+        themeExtensionFields: themeExtensionFields,


I'd say if there is annotation @themeExtension on the field we can assume it conforms to ThemeExtension<T> interface therefore can safely call lerp on it. I can see that it can be a bit problematic to determine type of class that is not yet generated (I think it's good idea to add annotation of field directly)

Suggested change

isAnotherTailorTheme: hasTailorComponentAnnotation,

isThemeExtension: isThemeExtension,

themeExtensionFields: themeExtensionFields,

isThemeExtension: isThemeExtension,

When it comes to the 'isAnotherTailorTheme' it should not matter 🤔

Rongix · 2022-06-14T06:03:36Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+      if (isThemeExtension) {
+        final fieldListingVisitor = _FieldListingVisitor();
+        coreType.element!.visitChildren(fieldListingVisitor);
+        themeExtensionFields.addAll(fieldListingVisitor.fieldNames);
+      }


Suggested change

if (isThemeExtension) {

final fieldListingVisitor = _FieldListingVisitor();

coreType.element!.visitChildren(fieldListingVisitor);

themeExtensionFields.addAll(fieldListingVisitor.fieldNames);

}

Rongix · 2022-06-14T06:03:46Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+
+      final isThemeExtension = extendsThemeExtension || hasTailorComponentAnnotation;
+
+      final themeExtensionFields = <String>[];


Suggested change

final themeExtensionFields = <String>[];

Rongix · 2022-06-14T06:07:57Z

packages/theme_tailor/example/lib/theme_extension_in_field.dart

+@tailor
+class $_SomeTheme {
+  static List<Color> appBackground = [AppColors.white, AppColors.black];
+  static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];


Suggested change

static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];

@themeExtension

static List<AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];

maybe even themeExtension will be better

Rongix

Must fix:
If list type is nullable is is encoded as non-nullable in theme extension (Previously we were using .getDisplayString(withNullability: true))

@Tailor
class $_SomeTheme {
static List<Color?> appBackground = [AppColors.white, AppColors.black];
}

Optionally to change:

Run _TailorClassVisitor first, get info which fields are marked with '@themeExtension'
Pass these fields names to _ListFieldTypeASTVisitor and get types

Rongix · 2022-06-14T13:10:32Z

packages/theme_tailor_annotation/lib/src/theme_tailor_component.dart

+import 'package:meta/meta_meta.dart';
+import 'package:theme_tailor_annotation/theme_tailor_annotation.dart';
+
+const tailorComponent = TailorComponent();


Needs documentation (Now I see that Tailor also lacks docs :O)
Maybe:
/// Default instance of [TailorComponent] annotation

Rongix · 2022-06-14T13:18:24Z

packages/theme_tailor_annotation/lib/src/theme_tailor_component.dart

+
+const tailorComponent = TailorComponent();
+
+@Target({TargetKind.classType})


Also can add docs here, maybe smth like:
"An annotation used to specify that generated ThemeExtension class can be used as a part of other ThemeExtension class and does not need additional extensions on BuildContext nor ThemeData for the ease of access"

Rongix · 2022-06-14T13:25:00Z

packages/theme_tailor_annotation/lib/theme_tailor_annotation.dart

@@ -3,3 +3,5 @@ library theme_tailor_annotation;
 export 'src/theme_encoder.dart';
 export 'src/theme_getter.dart';
 export 'src/theme_tailor.dart';
+export 'src/theme_tailor_component.dart';
+export 'src/theme_extension.dart';


No newline at end of file (please add one)

Rongix · 2022-06-14T13:33:33Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+  }
+}
+
+class ListFieldTypeASTVisitor extends SimpleAstVisitor {


Can be private

Rongix · 2022-06-14T13:43:27Z

packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

+      final extendsThemeExtension = coreType.typeImplementations
+              .firstWhereOrNull((t) => t
+                  .getDisplayString(withNullability: false)
+                  .startsWith('ThemeExtension')) !=
+          null;


Suggested change

final extendsThemeExtension = coreType.typeImplementations

.firstWhereOrNull((t) => t

.getDisplayString(withNullability: false)

.startsWith('ThemeExtension')) !=

null;

final extendsThemeExtension = coreType.typeImplementations.any((e) => e

.getDisplayString(withNullability: false)

.startsWith('ThemeExtension'));

…inor fixes

packages/theme_tailor_annotation/lib/src/theme_tailor_component.dart

Co-authored-by: Rongix <pawel.franitza@iteo.com>

jakubtiteo added 3 commits June 13, 2022 10:57

Add support for other theme extensions in class fields

8eba6fb

Change example naming

7ce4d97

Merge branch 'main' into feature/other_extensions_in_theme_class_fields

8ec63b8

# Conflicts: # packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart # packages/theme_tailor/lib/src/template/theme_class_template.dart

jakubtiteo added 4 commits June 13, 2022 16:49

Implement copyWith and lerp functions generating, add @TailorComponen…

c5164d6

…t annotation

Add code generation of static list of themes, that can be used for in…

94cffa8

…itial value

Merge branch 'main' into feature/other_extensions_in_theme_class_fields

e730509

# Conflicts: # packages/theme_tailor/lib/src/generator/theme_tailor_generator.dart

Fix merge errors

22bddaf

Rongix requested changes Jun 14, 2022

View reviewed changes

jakubtiteo added 3 commits June 14, 2022 13:15

Apply PR comments

9ff0e0c

Fix formatting and remove unnecessary changes

ac4ec44

Reformat code to keep 80 character per line limit

b6a6030

Rongix requested changes Jun 14, 2022

View reviewed changes

Get only required field types from AST, add documentation and other m…

b5737d9

…inor fixes

Rongix approved these changes Jun 15, 2022

View reviewed changes

packages/theme_tailor_annotation/lib/src/theme_tailor_component.dart Outdated Show resolved Hide resolved

Remove unnecessary code

6c7a363

Co-authored-by: Rongix <pawel.franitza@iteo.com>

Rongix merged commit 333952b into main Jun 20, 2022

Rongix deleted the feature/other_extensions_in_theme_class_fields branch June 29, 2022 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for other theme extensions in class fields #10

Add support for other theme extensions in class fields #10

jakubtiteo commented Jun 13, 2022 •

edited

Rongix commented Jun 13, 2022 •

edited

jakubtiteo commented Jun 13, 2022

Rongix commented Jun 13, 2022 •

edited

Rongix left a comment •

edited

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022 •

edited

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022 •

edited

Rongix left a comment •

edited

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022

Rongix Jun 14, 2022

	static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];
	static List<AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];


		final isThemeExtension = extendsThemeExtension \|\| hasTailorComponentAnnotation;

		final themeExtensionFields = <String>[];

	static List<$_AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];
	@themeExtension
	static List<AnotherThemePart> anotherThemePart = [AnotherThemePart.light, AnotherThemePart.dark];


		const tailorComponent = TailorComponent();

		@Target({TargetKind.classType})

Add support for other theme extensions in class fields #10

Add support for other theme extensions in class fields #10

Conversation

jakubtiteo commented Jun 13, 2022 • edited

Rongix commented Jun 13, 2022 • edited

jakubtiteo commented Jun 13, 2022

Rongix commented Jun 13, 2022 • edited

Rongix left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rongix Jun 14, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rongix Jun 14, 2022 • edited

Choose a reason for hiding this comment

Rongix left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubtiteo commented Jun 13, 2022 •

edited

Rongix commented Jun 13, 2022 •

edited

Rongix commented Jun 13, 2022 •

edited

Rongix left a comment •

edited

Rongix Jun 14, 2022 •

edited

Rongix Jun 14, 2022 •

edited

Rongix left a comment •

edited