daattali / colourpicker

🎨 A colour picker tool for Shiny and for selecting colours in plots (in R)
https://daattali.com/shiny/colourInput/
Other
217 stars 28 forks source link

"grey" (UK spelling) no longer works with new update #25

Closed nuno-agostinho closed 6 years ago

nuno-agostinho commented 7 years ago

Hey, @daattali! First of all, let me congratulate for colourInput. The new update is particularly great for allowing to set the transparency level.

However, with the new update, using the colour grey (UK spelling) no longer works, although gray still works as expected. This was a special annoyance for one R package of mine where grey points are plotted in a white background. As grey is not recognised anymore, those points are plotted in white instead.

daattali commented 6 years ago

Thanks for the report.

This is happening because previously the colours were translated in R, and now all the colour mapping is done in JS. For some reason, some colours were not in the colour mapping. I fixed that in the JS library here: https://github.com/daattali/jquery-colourpicker/commit/b5300fd9333860fb78f1193aba7747495f9f0460

But for some reason, trying to incorporate that into colourpicker results in another weird bug: now some colours don't work even though they are in the map. For instance, "gray" doesn't work ... so I'm going to hold off adding this to the package for now until that's resolved

daattali commented 6 years ago

@DavidRGriswold when I try to use the updated colourpicker JS library that contains all R colours, some colours such as all the grays don't work. I'm guessing it's because of something to do with the fact that there are multiple names for the same RGB value, but haven't confirmed that (haven't been able to pinpoint the problem yet).

Any ideas?

DavidRGriswold commented 6 years ago

Yeah, the issue is that the original object has the hex as the object name and the color name as the value. I have found the bug. I'm going to fix it in colourpicker by manually including both objects, just requires a little massaging. Should the "grey" or "gray" be the default?

daattali commented 6 years ago

Awesome! It doesn't matter which one gets preference

On Nov 13, 2017 09:06, "DavidRGriswold" notifications@github.com wrote:

Yeah, the issue is that the original object has the hex as the object name and the color name as the value. I have found the bug. I'm going to fix it in colourpicker by manually including both objects, just requires a little massaging. Should the "grey" or "gray" be the default?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/daattali/colourpicker/issues/25#issuecomment-343928647, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6IFGd-Jdb4U7KiWUACkz3CFesZxScYks5s2EzqgaJpZM4Qa-Kb .

daattali commented 6 years ago

@DavidRGriswold thanks for fixing it. Do you want to update the js files here as well? I prefer you do it so that you can get the credit in the git history. If you do, please remember to update the dependency from version 1.3 to 1.4 in inst/htmlwidgets/colourWidget.yaml

DavidRGriswold commented 6 years ago

I won't have time to do this for at least a couple of days. Go ahead and do it if you have time yourself; I'm not worried about credit.

On Nov 13, 2017 9:30 PM, "Dean Attali" notifications@github.com wrote:

@DavidRGriswold https://github.com/davidrgriswold thanks for fixing it. Do you want to update the js files here as well? I prefer you do it so that you can get the credit in the git history. If you do, please remember to update the dependency from version 1.3 to 1.4 in inst/htmlwidgets/colourWidget.yaml

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/daattali/colourpicker/issues/25#issuecomment-344135644, or mute the thread https://github.com/notifications/unsubscribe-auth/ATpU74nRPqEtd51qXuTDBPKV4_j9oX6dks5s2QlHgaJpZM4Qa-Kb .

daattali commented 6 years ago

thanks @DavidRGriswold

daattali commented 6 years ago

@nuno-agostinho please install the latest version from github and let me know if you still find an issue