hsv-alpha-color-picker-android icon indicating copy to clipboard operation
hsv-alpha-color-picker-android copied to clipboard

Default button sets a wrong color 888888

Open aloz77 opened this issue 5 years ago • 9 comments

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.

aloz77 avatar Apr 22 '19 06:04 aloz77

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.)

martin-stone avatar Apr 23 '19 19:04 martin-stone

@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 avatar Aug 24 '19 06:08 yvolk

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!

yvolk avatar Aug 24 '19 12:08 yvolk

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

aloz77 avatar Aug 25 '19 02:08 aloz77

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 avatar Aug 25 '19 08:08 yvolk

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

yvolk avatar Aug 25 '19 11:08 yvolk

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

aloz77 avatar Aug 26 '19 01:08 aloz77

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.

martin-stone avatar Sep 01 '19 19:09 martin-stone

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...)

yvolk avatar Sep 02 '19 12:09 yvolk