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

Package dynamic_color does not initialize new ColorScheme roles #582

Open
1 task done
amal-stack opened this issue Apr 17, 2024 · 5 comments
Open
1 task done

Package dynamic_color does not initialize new ColorScheme roles #582

amal-stack opened this issue Apr 17, 2024 · 5 comments
Labels
bug Something isn't working p: dynamic_color

Comments

@amal-stack
Copy link

Package

dynamic_color

Existing issue?

What happened?

Steps to reproduce:

  1. Switch to the beta channel and upgrade Flutter using:
flutter channel beta

and then:

flutter upgrade
  1. Create a new Flutter app.
  2. Add the dynamic_color package:
flutter pub add dynamic_color
  1. Use a platform where DynamicColorPlugin.getCorePalette does not return null (like Android).
  2. Use the DynamicColorBuilder widget and view its generated ColorScheme properties, especially the new color roles like primaryFixed surfaceContainer and so on. (done using Text widgets in the code sample below).

Code Sample

The following code uses DynamicColorBuilder and displays some of the generated colors using Text widgets.

import 'package:dynamic_color/dynamic_color.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});
  @override
  Widget build(BuildContext context) {
    return DynamicColorBuilder(
      builder: (light, dark) => MaterialApp(
        title: 'Flutter Demo',
        theme: ThemeData(
          colorScheme: light,
          useMaterial3: true,
        ),
        themeMode: ThemeMode.system,
        home: const MyHomePage(title: 'DynamicColorBuilder Demo'),
        )
      );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});  
  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {

  @override
  Widget build(BuildContext context) {
    final light = Theme.of(context).colorScheme;

    return Scaffold(
      backgroundColor: Theme.of(context).colorScheme.surfaceContainerLowest,
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
              child: Column(
                mainAxisSize: MainAxisSize.min,
              children: [
                Text('Primary ${light.primary}'),
                Text('Primary Fixed ${light.primaryFixed} ${light.primaryFixed == light.primary}'),
                Text('Primary Fixed Dim  ${light.primaryFixedDim} ${light.primaryFixedDim == light.primary}'),
                Text('Surface  ${light.surface}'),
                Text('Surface Container  ${light.surfaceContainer} ${light.surfaceContainer == light.surface}'),
              ],),
            ), 
    );
  }
}

Expected Results

Extension method CorePaletteToColorScheme.toColorScheme() should assign each of these new surface and fixed color roles.

Actual results

All the new color roles return the fallback colors. For instance, primaryFixed and primaryFixedDim return the fallback primary because this is the behavior of Flutter's ColorScheme as can be seen in its source:

 final Color? _primaryFixed;
  /// A substitute for [primaryContainer] that's the same color for the dark
  /// and light themes.
  Color get primaryFixed => _primaryFixed ?? primary;

  final Color? _primaryFixedDim;
  /// A color used for elements needing more emphasis than [primaryFixed].
  Color get primaryFixedDim => _primaryFixedDim ?? primary;

Reason

This is happening because DynamicColorBuilder converts a CorePalette to a Flutter ColorScheme using the extension method on CorePalette: CorePaletteToColorScheme.toColorScheme(). This method still uses the deprecated Scheme.lightFromCorePalette and Scheme.darkFromCorePalette methods and does not assign colors to the new roles.

I tried to manually write my own builder using the DynamicColorPlugin.getCorePalette method. However, the MCU library does mention an explicit way to convert a CorePalette to the new DynamicScheme type (so I created an issue here.

Relevant log output

No response

@amal-stack amal-stack added bug Something isn't working triage Issues that haven't been triaged labels Apr 17, 2024
@tomc128
Copy link

tomc128 commented Apr 18, 2024

Also facing this exact same issue. Would be great to get this fixed, as it renders these new colours unusable whilst using dynamic colour.

Looks like it may be related in some way to #574

@tomc128
Copy link

tomc128 commented Apr 27, 2024

For anyone wanting a quick workaround for now, this is how I solved it. For each of the lightDynamic and darkDynamic schemes generated from dyamic_color, I create a ColorScheme from seed, using each one's primary colour. Then, I simple take whichever colours I need from this scheme and set them in the dynamic scheme. Here's the code I used, if anyone would like to use it:

(ColorScheme light, ColorScheme dark) _generateDynamicColourSchemes(ColorScheme lightDynamic, ColorScheme darkDynamic) {
    var lightBase = ColorScheme.fromSeed(seedColor: lightDynamic.primary);
    var darkBase = ColorScheme.fromSeed(seedColor: darkDynamic.primary, brightness: Brightness.dark);

    var lightAdditionalColours = _extractAdditionalColours(lightBase);
    var darkAdditionalColours = _extractAdditionalColours(darkBase);

    var lightScheme = _insertAdditionalColours(lightBase, lightAdditionalColours);
    var darkScheme = _insertAdditionalColours(darkBase, darkAdditionalColours);

    return (lightScheme.harmonized(), darkScheme.harmonized());
  }

  List<Color> _extractAdditionalColours(ColorScheme scheme) => [
        scheme.surface,
        scheme.surfaceDim,
        scheme.surfaceBright,
        scheme.surfaceContainerLowest,
        scheme.surfaceContainerLow,
        scheme.surfaceContainer,
        scheme.surfaceContainerHigh,
        scheme.surfaceContainerHighest,
      ];

  ColorScheme _insertAdditionalColours(ColorScheme scheme, List<Color> additionalColours) => scheme.copyWith(
        surface: additionalColours[0],
        surfaceDim: additionalColours[1],
        surfaceBright: additionalColours[2],
        surfaceContainerLowest: additionalColours[3],
        surfaceContainerLow: additionalColours[4],
        surfaceContainer: additionalColours[5],
        surfaceContainerHigh: additionalColours[6],
        surfaceContainerHighest: additionalColours[7],
      );

and the actual DynamicColorBuilder widget:

return DynamicColorBuilder(
      builder: (ColorScheme? lightDynamic, ColorScheme? darkDynamic) {
        ColorScheme lightScheme, darkScheme;

        if (lightDynamic != null && darkDynamic != null) {
          (lightScheme, darkScheme) = _generateDynamicColourSchemes(lightDynamic, darkDynamic);
        } else {
           // logic to set standard static themes here
        }

        return MaterialApp(...);
      },
    );

@guidezpl
Copy link
Collaborator

guidezpl commented Jul 2, 2024

The solution I think will work is described in #574 (comment)

@noahpolimon
Copy link

noahpolimon commented Jul 4, 2024

For anyone wanting a quick workaround for now, this is how I solved it. For each of the lightDynamic and darkDynamic schemes generated from dyamic_color, I create a ColorScheme from seed, using each one's primary colour. Then, I simple take whichever colours I need from this scheme and set them in the dynamic scheme. Here's the code I used, if anyone would like to use it:

Thank you very much, I'm new to Flutter and M3 so I can't think of these hacks yet (or maybe i'm just too lazy). However, while your solution is correct, you are extracting colors from the generated scheme then reinserting them into the same scheme, which is unnecessary.

here is my solution for now:

 ColorScheme _generateColorScheme(Color? primaryColor,
      [Brightness? brightness]) {
    final Color seedColor = primaryColor ?? kFallbackAccentColor;

    final ColorScheme newScheme = ColorScheme.fromSeed(
      seedColor: seedColor,
      brightness: brightness ?? Brightness.light,
    );
  }

  return newScheme.harmonized();
}

then use it as follows:

return DynamicColorBuilder(
      builder: (lightDynamic, darkDynamic) {
        final ColorScheme lightScheme = _generateColorScheme(lightDynamic?.primary);
        final ColorScheme darkScheme = _generateColorScheme(darkDynamic?.primary, Brightness.dark);

        return MaterialApp(...);
      },
);

@OliverRhyme
Copy link

#574 (comment)

shouldn't you be inserting to the original color scheme passed to the function (which lacks the new color roles)? I think the current solution is redundant as you are getting and setting the new color roles to the same color scheme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p: dynamic_color
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants