Closed Olganix closed 1 year ago
May be it miss a option to not display tooltip.
Swatches can be modified by settings the swatches
option, but I have no intention of introducing a UI to add and remove swatches.
On the other hand, your PR has a few problems:
Unfortunately, I'm going to have to reject your PR. In the feature, please create an issue first and explain what you intend to do before submitting a PR to avoid wasting your time unnecessarily.
"Swatches can be modified by settings the swatches option, but I have no intention of introducing a UI to add and remove swatches." => Ok. as you want.
"On the other hand, your PR has a few problems:" "You don't adhere to the same coding style" => I have tryed to follow what you have done. I used to use jquery witch make it simpliest. but here, I keep pure javascript, take yours way to name, add options like previous ones.
"You replicated code that isn't necessary for the feature you are adding" => I'm pretty sure all lines is necessary (may be some security check could be removed, but ...)
"You modified the whole HTML block that generates the color picker's UI instead of only adding the extra lines you need" => I'm adding only one color swatch's button on "add" button click, So i don't understand what you said. The setOption({ swatches: ... ]) is yours code, I didn't change it.
"You don't need an extra input field, there is already one in the dialog" => It's for the name of color (I need it for my project, may be could interrest some people witch use it.
"The whole implementation is inefficient for various other reasons" => Ok if you say so.
"Unfortunately, I'm going to have to reject your PR." => No problem.
"In the feature, please create an issue first and explain what you intend to do before submitting a PR to avoid wasting your time unnecessarily." => I'm not a expert of GitHub conventions. I always work on it with people witch I could talk directly, Sorry for that. I have coding this because I need it. And I thinking it was a feature witch could interress someone. It take me less of 1h to read yours code and modify it with following conventions/way to do. It take me more time to try to push / talk about it.
It just add a edition of swatches (only on src, not dist).
My modifications : -deal with a swatches list = [ {color: "#FF00FF", name: "Color1"} , etc ... ] (use direct "#FF00FF" still continue to work). -add a tooltip with the name under the swatch color button, on hover. -add a input (for name edition) + a "Add" button to add a swatch (label and internationnalization is keeped like other buttons). -use right-click on a swtch color button to delete it. -add a onSwatchesChange (used like onChange) to get the list of swatches (rebuilded from html buttons).