search

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

Fix 49 #50

Closed JulianEggers closed 5 years ago

JulianEggers commented 5 years ago

Here is my proposal for fixing issue #49.

JulianEggers commented 5 years ago

Hi Martin, is the solution I provided basically ok and has the potential to be merged into the next release? If so, can I assume that the next release will be in the near future? Otherwise I have to think about another solution to fix this behaviour. Thanks for the awesome color picker Julian

martin-stone commented 5 years ago

Hi Julian. Apologies for the slow response.

I assume you're using the ColorPickerView directly and using and storing the HSV values yourself, rather than using the ColorPreference? It looks like these changes would have no effect on the Preference side of things.

It looks fine to merge. There is the question of which branch(es) to merge to -- You may be aware that I've been hoping to switch over to a pure app-compat fragment codebase. I'd expected by now that I'd have had some feedback on this and be able to maintain a single version. Where do you stand on app-compat?

JulianEggers commented 5 years ago

Hi Martin. No worries. I do not use the color picker with preferences and I am not going to do so in future. My App therefore is completely unaffected by the recent changes of v3.0.0.

I am using the PreferenceFragmentCompat in my app and would in general therefore support a pure app-compat fragment codebase.

Therefore, I think we should merge my changes into the support-lib-preferences branch and then work towards a pure app-compat fragment codebase.

I did not work with app compat preferences much but I will take look at the changes and maybe I can provide some feedback. (Update: 3.0.0 is fine for me.)

~~Without a closer look, I saw the new implementation requires to override onDisplayPreferenceDialog - is this necessary? I would expect the color picker preference to handle that out of the box.~~ Seems like this is how it is done with app-compat preferences.

martin-stone commented 5 years ago

Seems like this is how it is done with app-compat preferences.

Yeah :-( This sort of ugliness is why I've been dragging me feet over this.