yukuku / ambilwarna

Android Color Picker aka AmbilWarna library ("Pick a Color" in Indonesian)
Other
222 stars 88 forks source link

Added OnChange listener #16

Open alexnix opened 8 years ago

alexnix commented 8 years ago

I had the need to know to color the user selected in real time (while he is dragging the color selector) that is before onOk callback gets called so I implement an onChange callback that is called instantly when the user choses a color.

denisk20 commented 8 years ago

Hi, thanks for your contribution! Do you think that real-time color update is a general scenario which should go into the core library? Sounds more like an edge use case which can be useful for a particular project, but which will just provide an extra empty method to implement for the majority of people.

alexnix commented 8 years ago

You are right that could be some overhead code for most use cases. What do you say if I add another interface OnAmbilWarnaLiveListener that extends OnAmbilWarnaListener and this one has all three methods? Just in case anyone else is ever looking for this same functionality...

denisk20 commented 8 years ago

It makes sense, but one thing that worries me is that onChange listener is called so many times during color selection, so it can easily become a performance bottleneck. For this feature's potential consumers I would recommend simply putting a comment into existing OnAmbilWarnaListener which would suggest that it is also possible to add onChange method to the interface and briefly describe a place in AmbilWarnaDialog where such method would be called (you could also put a comment inside AmbilWarnaDialog itself, something like //onChange would be called here. The comment should also warn a potential user about performance deterioration that the implementation of onChange may lead to.