martin-stone / hsv-alpha-color-picker-android

A color picker and a color preference for use in Android applications.
Apache License 2.0
290 stars 60 forks source link

Default button sets a wrong color 888888 #51

Open aloz77 opened 5 years ago

aloz77 commented 5 years ago

Using colorpicker 2.4.2 with Android 7 device.

<de.myapp.ColorPickerPref
                android:defaultValue="#000000"
                android:key="appLauncherTextColor"
                android:summary="Select app label color"
                android:title="Launcher Text Color"
                app:colorpicker_selectNoneButtonText="Default"
                app:colorpicker_showAlpha="false" />

Pressing the Default button in the colorpicker dialog box leads to showing 888888 as selected color and the thumbnail on the pref list is colored darkgray.

Expect setting color to 000000.

Actually colorpicker is still passing the correct value #000000 back to settings. However the view in the colorpicker dialog box and pref list is misleading.

martin-stone commented 5 years ago

Are you able to use the support library preferences in your app? There is a beta version 3.0.0, which I believe no longer has this issue. In any case, I'm hoping to maintain a single version, so ideally any bug fixes would go into the new support library version. If you do switch over, please let me know if it's all working for you properly. (My own apps are still on the native preferences so I haven't dogfooded v3 as much as I'd like.)

yvolk commented 5 years ago

@martin-stone I encountered the same problem in v.2.4.2 trying to migrate our ToDo Agenda app ( https://github.com/plusonelabs/calendar-widget ) to this Color Picker. Very sad bug...

Migration of our app to Support library is time consuming AND, what's more important, it's too late to migrate to com.android.support:preference etc. - it's no longer updated. If migrate then migrate to androidx.preference... ?!

yvolk commented 5 years ago

In order to fix this bug I added the library's source code directly to my application and made changes, similar to what was done in the branch support-lib-preferences... https://github.com/andstatus/todoagenda/blob/more-colors/colorpicker/src/main/java/com/rarepebble/colorpicker/ColorPreference.java It works, thank you!

aloz77 commented 5 years ago

Martin: Is there a chance to fix it in the Colorpicker repository?

yvolk commented 5 years ago

Playing now with Default button, I see that it needs other behavior: if the defaultValue for the preference is set, pressing "Default" button should set selected color to the default value, but shouldn't close the Color picker view. So a User will be able to compare previous color with the default one. These are "OK" or "Cancel" buttons that should close the view and change (or not) the preference's value ?!

yvolk commented 5 years ago

I implemented the new Default button behavior, see above commit.

aloz77 commented 5 years ago

@yvolk: Thank you, it looks likes a very straightforward fix. @martin-stone: Is there a chance to fix it in Colorpicker?

martin-stone commented 5 years ago

Your default button behaviour seems reasonable, although it causes Default and No Color functions to behave differently. Possibly confusing for users if an application uses both features but maybe that's the application's problem.

However, I would like to stick to maintaining a single branch. At the time of updating to support preferences, AndroidX was still in alpha, but it looks like you can still use this library and its 28.0.0 dependencies with AndroidX 1.0.0.

yvolk commented 5 years ago

Thank you, @martin-stone I will try to use updated version of the Color Picker when it's ready.

Regarding "No color" feature/Button, I think that it should be implemented the same way as the new Default button behavior, i.e. a visual clue for a User that now "OK" will mean "No color"... I don't need such a feature right now, this is why I didn't implement it (mainly, we need to decide, how "No color" should be presented to the User).

Oh, BTW "No color" may conceptually come as an input also. So the Color Picket should have a way to be told so (Java 8 has Optional for this...)