-
Notifications
You must be signed in to change notification settings - Fork 46
Refactor gradient color handling to use color tokens #773
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
base: main
Are you sure you want to change the base?
Conversation
Updated gradient mix classes to accept lists of color property tokens instead of direct color lists, enabling dynamic color resolution. Example usage now demonstrates color tokens for primary and secondary colors, improving theme flexibility and consistency.
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors gradient color handling in the Mix package to support dynamic color tokens instead of static color lists. This enables better theme flexibility and consistency by allowing gradient colors to be resolved at runtime from token-based color systems.
- Changed gradient mix classes to accept lists of color property tokens (
List<Prop<Color>>) instead of direct color lists - Updated test assertions to verify individual color token resolution rather than list-based comparisons
- Modified example to demonstrate usage with
ColorTokeninstances andMixScopefor theme management
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/mix/lib/src/properties/painting/gradient_mix.dart |
Refactored core gradient classes to handle color tokens, updated property types and resolution logic |
packages/mix/test/src/properties/painting/gradient_mix_test.dart |
Updated test expectations to verify individual color token resolution patterns |
packages/mix/example/lib/api/gradients/gradient_linear.dart |
Added example demonstrating color tokens with MixScope for theme-based gradient colors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $colors?.map((e) => MixOps.resolve(context, e)!).toList() ?? | ||
| defaultValue.colors; | ||
|
|
||
| return LinearGradient( | ||
| begin: MixOps.resolve(context, $begin) ?? defaultValue.begin, | ||
| end: MixOps.resolve(context, $end) ?? defaultValue.end, | ||
| colors: MixOps.resolve(context, $colors) ?? defaultValue.colors, | ||
| colors: colors, |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-null assertion operator (!) on MixOps.resolve(context, e) could cause a runtime exception if resolution returns null. Consider handling the null case explicitly or document why null resolution is impossible here.
| final colors = | ||
| $colors?.map((e) => MixOps.resolve(context, e)!).toList() ?? | ||
| defaultValue.colors; |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-null assertion operator (!) on MixOps.resolve(context, e) could cause a runtime exception if resolution returns null. Consider handling the null case explicitly or document why null resolution is impossible here.
| final colors = | |
| $colors?.map((e) => MixOps.resolve(context, e)!).toList() ?? | |
| defaultValue.colors; | |
| final colors = $colors != null | |
| ? $colors | |
| .map((e) => MixOps.resolve(context, e)) | |
| .whereType<Color>() | |
| .toList() | |
| : defaultValue.colors; |
| @override | ||
| SweepGradient resolve(BuildContext context) { | ||
| final colors = | ||
| $colors?.map((e) => MixOps.resolve(context, e)!).toList() ?? |
Copilot
AI
Oct 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-null assertion operator (!) on MixOps.resolve(context, e) could cause a runtime exception if resolution returns null. Consider handling the null case explicitly or document why null resolution is impossible here.
| $colors?.map((e) => MixOps.resolve(context, e)!).toList() ?? | |
| $colors?.map((e) => MixOps.resolve(context, e)).whereType<Color>().toList() ?? |
Description
Updated gradient mix classes to accept lists of color property tokens instead of direct color lists, enabling dynamic color resolution. Example usage now demonstrates color tokens for primary and secondary colors, improving theme flexibility and consistency.
Changes
List specific changes made in this PR.
Review Checklist
Additional Information (optional)
Is there any additional context or documentation that might be helpful for reviewers?