search

rydmike / flex_color_picker

A highly customizable Flutter color picker.
BSD 3-Clause "New" or "Revised" License
200 stars 42 forks source link

`showColorPickerDialog` could reuse the theme when possible #63

Closed ValentinVignal closed 2 years ago

ValentinVignal commented 2 years ago

For example, the value of actionsPadding is set by default to const EdgeInsets.symmetric(horizontal: 16), but it could use Theme.of(context).dialogTheme.actionsPagging https://api.flutter.dev/flutter/material/DialogTheme/actionsPadding.html

https://github.com/rydmike/flex_color_picker/blob/6af74ef68a8b9c8533c374fc324372ffae4f63a2/lib/src/show_color_picker_dialog.dart#L476-L484

rydmike commented 2 years ago

Totally agree with you, it should do that, buttonPadding as well.

Not even sure why they don't do that. There was probably some ancient opinionated default reasoning around it that probably made sense at the time, but i does not really do so anymore even in my own opinion.

I can make the props nullable, query the theme and use that before falling to current defaults without breaking it.

Although I'm almost leaning more towards just having them as null defaults and it gets theme defaults via AlertDialog instead, which uses the theme if it has one. Plus it uses correct different default2 in Material 2 and Material 3 mode, if I do that. Although that would visually be a small style break with past default style, I don't think it matters so much though. I will compare the results, maybe it is OK to break visually a bit with past style.

rydmike commented 2 years ago

Feature and change included in version 3.0.0, see https://pub.dev/packages/flex_color_picker/changelog