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

White colour selects multiple colour #66

Closed
GAayushi opened this issue Jun 19, 2023 · 5 comments · Fixed by #68
Closed

White colour selects multiple colour #66

GAayushi opened this issue Jun 19, 2023 · 5 comments · Fixed by #68
Assignees
Labels
enhancement New feature or request
Projects
Milestone

Comments

@GAayushi
Copy link

I've created a custom colour library. While selecting some light colours and white colour, if we select white colour from the material tone palette, multiple colours are selected. Please find the screenshot.
white_color_select

@rydmike
Copy link
Owner

rydmike commented Jun 19, 2023

Hi @GAayushi, thanks for the report.

Can you provide a code sample including the custom MaterialColors you used?

Even without it, here is what I suspect is going on.

If you set the picker to show the "old" Material2 style palettes enebaleShadesSelection: true, I am pretty sure you can see/show that you have white color in all the above selected top level custom MaterialColor palettes.

If a selected color exists in more than one MaterialColor palette, in the same picker tab, the ColorPicker will select the main color [500], in all palettes that contains the same selected color value, to indicate which other palettes the same color value exists in. In this case you have white included in all of the 5 first MaterialColor palettes.

The actual color selection of "white" in this case, if you paste in the white value, would be selected from the first MaterialColor, where it finds it, being custom yellow[500] in this case.

If you enable the MaterialColor palette shades with these custom colors, that apparently all contain white, and select "white" in them from any other palette than yellow, you will see that it changes the sub palette to be from the yellow one, where it first finds the white color among all the white color values in this picker tab.

The situation you show above, can however be reached by selected the top blue color and then clicking on white in its tonal palette.

FlexColorPicker is still focused mainly on Material2 style palettes, where its source colors use the indexes from 50 too 900, and that do not even in a correct MaterialColor contain white or black color, unless you deal with a custom greyscale.

If you generated the custom MaterialColors used above with the "faux" ColorTools.createPrimarySwatch it may also generate MaterialColor where index [50]is white. The actual Material2 algorithm used to create the original MaterialColor palettes is not public, like the new Material3 algorithm, so it is just a fake version that produces similar styled colors. but where [50] may become whit if source [500] color is very light, and [900] can also become dark if source is very dark.

Of course making custom MaterialColor that includes same color(s) in different palettes, like white or even other colors, that clashes and contain duplicates is totally possible, but it was not a part of the original MaterialColor spec to do so. This is a part of the reason why it behaves this way when there are duplicates of same color value in other palettes.

When using Material3 tonal palettes the situation is totally different, as a tonal palette starts from index 0 and it is always fully black and ends in index 100, being fully white, with then varying Chroma and Hue on the tones in between.

Currently FlexColorPicker has very limited support for M3 tonal palettes, but it is on my todo list to add that, by adding an own Tonal Palette color picker tab. However, such a release would probably be major new version and break some pasts older features. Since I would probably clean feature to show tonal palettes at same time with MaterialColor, since that is a bit confusing and conflicting.


It might be possible to add a small fix to not show a check mark on other palettes containing selected color, when using a setup where the selected Material shades are not shown at all and showing only the Tonal Palette, like you do here, so that it would not indicate the other palettes where the color exists.

Currently this is behaving as expected, but when not showing the MaterialColor shades and only using Tonal Palette is admittedly a bit confusing and not very logical, the behavior could certainly be improved/changed.

I may look into this.

It might also be something I don't add until I do major feature bump on the ColorPicker. It is way over due for a major feature bump.

@GAayushi
Copy link
Author

Hello, Please take a look of the code, I'm currently using. I'm allowing user to choose the colour form the wheel, and with those selected colour, I'm creating Swatches. Also, I've used ColorTools.createPrimarySwatch for creating user-defined colour library. Here is my code.

class NewColorPicker extends StatelessWidget {
  final Function(String) _changedColor;
  final RxString color;
  final TextEditingController colorEditController = TextEditingController();
  final ReactiveRecord model;
  final designController = Get.put(DesignController());
  final RxBool showCustomLibrary;

  NewColorPicker(
    this._changedColor,
    this.color,
    this.model,
    this.showCustomLibrary,
  );
  @override
  Widget build(BuildContext context) {
    var customSwatchList = colorMap(designController.model.value!.colorList);
    colorEditController.text = color.value;
    return Obx(() => Column(
          crossAxisAlignment: CrossAxisAlignment.center,
          children: [
            SizedBox(
              width: double.infinity,
              child: Padding(
                padding: const EdgeInsets.all(6),
                child: Card(
                  elevation: 2,
                  child: ColorPicker(
                    // Use the screenPickerColor as start color.
                    color: HexColor.fromHex(color.value) ?? const Color(0xff4C45D6),
                    // Update the screenPickerColor using the callback.
                    onColorChanged: (Color color) => changeColor(color),
                    width: 44,
                    height: 44,
                    heading: Text(
                      'select_color'.tr,
                      style: Theme.of(context).textTheme.headlineSmall,
                    ),
                    enableShadesSelection: false,
                    wheelWidth: 24,
                    wheelSquarePadding: 8,
                    wheelSquareBorderRadius: 8,
                    enableTonalPalette: true,
                    enableTooltips: true,
                    copyPasteBehavior:
                        ColorPickerCopyPasteBehavior(copyFormat: ColorPickerCopyFormat.hexRRGGBB, snackBarMessage: 'Code copied successfully'),
                    tonalSubheading: Text('Material 3 tonal palette'),
                    pickersEnabled: <ColorPickerType, bool>{
                      ColorPickerType.wheel: true,
                      ColorPickerType.custom: showCustomLibrary.value,
                      ColorPickerType.accent: false,
                      ColorPickerType.primary: false
                    },
                    customColorSwatchesAndNames: customSwatchList,
                    showColorCode: true,
                    showColorName: true,
                    colorCodeHasColor: true,
                    pickerTypeLabels: <ColorPickerType, String>{ColorPickerType.custom: 'Your Colour Library'},
                  ),
                ),
              ),
            ),
            SizedBox(
              height: 30,
            ),
            AdminFormSubmitButtons(
              onPressedAction: onPressedActionHandler,
              firstButtonText: 'okay_text'.tr,
              secondButtonText: GetStringUtils('cancel_text'.tr).capitalize!,
            ),
          ],
        ));
  }

  /// Buttons click handler
  void onPressedActionHandler(ActionType actionType) {
    switch (actionType) {
      case ActionType.FirstButtonClick:
        if (UIUtil().isValidColor(color.value)) _saveColor(); // On Save
        break;
      case ActionType.SecondButtonClick:
        Get.back(); // On Cancel
        break;
      default:
    }
  }

  void changeColor(Color newColor) {
    color.value = newColor.toHex();
    colorEditController.text = color.value;
  }

  void _saveColor() {
    _changedColor(color.value);
    Get.back();
  }

  Map<ColorSwatch<Object>, String> colorMap(List<String> colorSwatchList) {
    var customSwatch = <ColorSwatch<Object>, String>{};
    for (var colors in colorSwatchList) {
      print('colorMap $colors');
      customSwatch[ColorTools.createPrimarySwatch(HexColor.fromHex(colors)!)] = '';
    }
    return customSwatch;
  }
}

@rydmike rydmike added enhancement New feature or request and removed in triage labels Jul 2, 2023
@rydmike rydmike added this to To do in TODOs via automation Jul 2, 2023
@rydmike rydmike self-assigned this Jul 2, 2023
@rydmike rydmike moved this from To do to In progress in TODOs Jul 2, 2023
@rydmike
Copy link
Owner

rydmike commented Jul 2, 2023

I have a behavior enhancement, or fix if you so prefer to call it, for this use case. I need to test it a bit more still. I will use your sample to test it as well. Thanks for submitting it. I might bundle this improvement/fix with a few other ones before the release. Of course, if you desperately/urgently need it I can release it in its own fix version as well.

@GAayushi
Copy link
Author

GAayushi commented Jul 6, 2023

Hy, It would be great if we could have that fix earlier.

rydmike added a commit that referenced this issue Jul 18, 2023
rydmike added a commit that referenced this issue Jul 18, 2023
@rydmike rydmike mentioned this issue Jul 18, 2023
rydmike added a commit that referenced this issue Jul 18, 2023
rydmike added a commit that referenced this issue Jul 18, 2023
@rydmike
Copy link
Owner

rydmike commented Jul 29, 2023

Hi @GAayushi, thanks for raising this issue. Hope you have had a chance to try it. There might be some edge cases where the solution I made for this might not be ideal. Hopefully you won't be running in to those. Anyway, let my know how the fix has worked out for you or if there is still some edge case with it for your usage.

@rydmike rydmike modified the milestones: 3.3.1, 3.3.0 Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Released
TODOs
In progress
Development

Successfully merging a pull request may close this issue.

2 participants