Pyozer / flutter_material_color_picker

Material color picker, you can customize colors. Selection in two step, first main color and after shades.
https://pub.dartlang.org/packages/flutter_material_color_picker
MIT License
75 stars 38 forks source link

_onMainColorSelected calls both onMainColorChange and onColorChange #4

Closed stanchanpsu closed 5 years ago

stanchanpsu commented 5 years ago

https://github.com/Pyozer/flutter_material_color_picker/blob/master/lib/src/material_color_picker.dart#L97

I don't think is ideal behavior because onColorChange will be triggered before the specific shade Color is even selected.

For example, I want to Navigator.pop() the color onColorChange and the dialog is closed early because onColorChange is triggered _onMainColorSelected()

Pyozer commented 5 years ago

I understand but, when you choose the main color, a shade color is automatically selected. Calling the onColorChange is useful when you use this color picker in a dialog, if the user choose the main color and after submit the dialog without select the shade color.

You can wrap it, store the current shade color and if the dialog is submit (and not canceled), return the color. You can check how I use it here: https://github.com/Pyozer/MyAgenda_Flutter/blob/master/lib/widgets/settings/list_tile_color.dart

Instead of remove the call of onColorChange when mainColor is chosen, we can add a parameter to call it or not.

If the onColorChange is not calling after the mainColor and the user just submit the dialog after chosen the mainColor, the color will be lost.

(Sorry if my english is not good 😆)

stanchanpsu commented 5 years ago

Hey that makes sense. And your English is fine, never apologize.

Actually, what do you think about adding a property allowShades: bool so that the material_color_picker can be used with only the main colors if needed. We can then use the flag to determine the behavior of the callbacks too.

Pyozer commented 5 years ago

Yeah it's a great idea, I will merge your PR ;)

stanchanpsu commented 5 years ago

Awesome, I've updated the docs and change list. Let me know if there's any issues with it.

Pyozer commented 5 years ago

Thank you for your PR, I merged it and publish new version to pub ;) I also made a small fix for mainColor with selectedColor parameter. Also, I add an example project with the package ;)

stanchanpsu commented 5 years ago

Awesome. I can't wait to use it.