Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

custom element #96

Closed
lekoala opened this issue Apr 24, 2023 · 4 comments
Closed

custom element #96

lekoala opened this issue Apr 24, 2023 · 4 comments

Comments

@lekoala
Copy link

lekoala commented Apr 24, 2023

hi there,

great library!

if you are interested, i've quickly put together a custom element
https://gist.github.com/lekoala/233b0c6246170716c52dbfab342caf22

by the way, the current api is not super friendly when you want to init each element on their own (i would rather have a new Coloris(myElem, options) type of api with maybe some kind of helper init function that loops over elements).

Also, you cannot have multiple visible inline elements due to the fact that there is only one picker. no big deal since i don't use them, but still something to keep in mind (and should maybe be added in the readme?).

@mdbassit
Copy link
Owner

hi there,

great library!

if you are interested, i've quickly put together a custom element https://gist.github.com/lekoala/233b0c6246170716c52dbfab342caf22

Maybe @melloware would be interested since they maintain the NPM/Typescript fork.

by the way, the current api is not super friendly when you want to init each element on their own (i would rather have a new Coloris(myElem, options) type of api with maybe some kind of helper init function that loops over elements).

I created this color picker partly because every other one I could find did what you just described, and I didn't want that. If that's what you need I suggest you use a different color picker.

Also, you cannot have multiple visible inline elements due to the fact that there is only one picker. no big deal since i don't use them, but still something to keep in mind (and should maybe be added in the readme?).

The README does state that there is only one true instance of the color picker and therefore it's not possible to show multiple instances at the same time. I'll see if can make it even clear. Thanks for pointing it out.

@lekoala
Copy link
Author

lekoala commented Apr 25, 2023

ok thanks :)

i think both approaches are compatible: only one instance for a given set of options. that means the picker can be reused as much as possible. but restricting to only one true instance means that you cannot show both pickers, which kind of defeat the inline feature in my opinion (i don't use it, so i don't care, but still, it's a bit odd)

@mdbassit
Copy link
Owner

This an intentional design choice to keep the performance of the color picker in check. This is used in a web app among dozens of other UI tools, and keep in it as light as possible is my number one priority.

I too don't use the inline mode, but I agreed to add it because it doesn't cost me much in terms of performance.

Anyway, this tool does things in a certain way, and it either works for you or it doesn't. I'm not interested in discussing the merits of alternative approches :)

@melloware
Copy link
Contributor

@lekoala I did something similar to wrap the component as a Java Server Faces component for PrimeFaces: https://github.com/primefaces/primefaces/blob/master/primefaces/src/main/resources/META-INF/resources/primefaces/colorpicker/1-colorpicker.js

I think we should just leave the current color picker as is and just use wrappers like we are doing. Safe to close this ticket.

@mdbassit mdbassit closed this as not planned Won't fix, can't repro, duplicate, stale Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants