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

[dynamic_color] Add new colors #589

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MahanRahmati
Copy link
Contributor

@MahanRahmati MahanRahmati commented May 17, 2024

Description

Add new material colors introduced in Flutter 3.22.

Issues

Fixes #582

Checklist

@MahanRahmati MahanRahmati requested a review from a team as a code owner May 17, 2024 23:33
@MahanRahmati MahanRahmati requested review from guidezpl and removed request for a team May 17, 2024 23:33
@MahanRahmati MahanRahmati marked this pull request as draft May 17, 2024 23:48
@MahanRahmati MahanRahmati changed the title Add new colors [dynamic_color] Add new colors May 18, 2024
@MahanRahmati
Copy link
Contributor Author

We need to bump material_color_utilities, and it is an SDK dependency.

@boredcodebyk
Copy link

boredcodebyk commented May 18, 2024

new colors doesn't seem to be added in Scheme and they apparently moved to DynamicScheme as Scheme is deprecated (see this)

this package seem to direct take CorePalette from android. I tried to make following changes locally according to their migration guide

// corepalette_to_colorscheme.dart
final SchemeTonalSpot scheme;
    
    switch (brightness) {
      case Brightness.light:
        scheme = SchemeTonalSpot(
            sourceColorHct: Hct.fromInt(primary.keyColor.toInt()),
            isDark: false,
            contrastLevel: 0.0);
        break;
      case Brightness.dark:
        scheme = SchemeTonalSpot(
            sourceColorHct: Hct.fromInt(primary.keyColor.toInt()),
            isDark: true,
            contrastLevel: 0.0);
        break;
    }
...
return ColorScheme(
      primary: Color(MaterialDynamicColors.primary.getArgb(scheme)),
      onPrimary: Color(MaterialDynamicColors.onPrimary.getArgb(scheme)),
...

but the unit test fails

dynamic_color: Core palette detected.
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Color:<Color(0xff286c2a)>
  Actual: Color:<Color(0xff8c4a60)>

for reference I used code from ColorScheme.fromSeed() from Flutter SDK

@andr202
Copy link

andr202 commented May 19, 2024

@boredcodebyk

but the unit test fails

dynamic_color: Core palette detected.
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Color:<Color(0xff286c2a)>
  Actual: Color:<Color(0xff8c4a60)>

for reference I used code from ColorScheme.fromSeed() from Flutter SDK

Shouldn't just correcting expecting colors in test file enough if everything else works perfectly? (not based on this error but checking what's the real output of given colorscheme in test on md3 colorscheme generator, as colorscheme colors changed, now we need to change test file too)

@boredcodebyk
Copy link

boredcodebyk commented May 19, 2024

@andr202
you have a valid point
but theoretical the test doesn't needs to be changed. in the test file a container is given primary color and tested with the expected color which passes. in this case, the primary color gets way off from expected color for some reason. I'm still looking into it (and now I'm clueless)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package dynamic_color does not initialize new ColorScheme roles
3 participants