simonwep / pickr

🎨 Flat, simple, multi-themed, responsive and hackable Color-Picker library. No dependencies, no jQuery. Compatible with all CSS Frameworks e.g. Bootstrap, Materialize. Supports alpha channel, rgba, hsla, hsva and more!
https://simonwep.github.io/pickr
MIT License
4.29k stars 287 forks source link

Inline mode trigger click on parent element #243

Closed bjarnef closed 4 years ago

bjarnef commented 4 years ago

What is the current behavior?

I have used Pickr as a color picker property editor in Umbraco CMS, which seems to work very well. However on a document type where a property is added it shows a preview of this (Pickr is rendered in background).

image

When inline mode is false it works as expected. However when inline mode is true it seems to trigger a click event on the parent element after the inline color picker is shown. So after the inline picker is rendered it opens this overlay, which usually only open when clicking the preview area. When inline mode in the Pickr instance is false this doesn't happen, so I am wondering a click event happens after rendering the inline picker?

image

Your environment:

Version (see Pickr.version): 1.7.2
Used bundle (es5 or normal one): Normal one
Used theme (default is classic): Classic
Browser-version:  Chrome v84.0.4147.135
Operating-system:  Windows 10
simonwep commented 4 years ago

Hmm, pickr doesn't do anything on it's one - especially not emitting UI-Events such as clicking stuff. I just checked the source and found nothing related to inline and clicking the parent element. The only thing it does (and always did) is mounting the widget directly after el which is in this case the child list of the parent-element.

Maybe you've discovered an edge-case? Would it be possible to break this down in a JSFiddle?

bjarnef commented 4 years ago

Sounds really strange since it only happens when inline mode is set to true. I could try copy the part of the markup from Umbraco and see if I can reproduce the issue in a JSFiddle.

bjarnef commented 4 years ago

@Simonwep okay, so I have made a JSFiddle for you, where I have copied the markup from Umbraco, but omitted some angular logic to keep it simple. https://jsfiddle.net/3ukozf89/8/

So inside this "preview" container the Pickr instance is rendered.

Umbraco then have an angular click event on this preview container and a separate settings buttons. I have added click events on these with id "preview" and "settings".

When Picker has inline: false there are no issues. However with inline: true it trigger the click event on element with preview id.

document.getElementById("settings").addEventListener("click", displayDate);
document.getElementById("preview").addEventListener("click", displayDate);

function displayDate() {
  document.getElementById("test").innerHTML = Date();
}

image

If I comment out this line the date is not rendered in the output in element with id test.

document.getElementById("preview").addEventListener("click", displayDate);

So it seems something in Pickr must fire a click on this ancestor element since I would expect it only to trigger when clicking this event. If I comment out the aforementioned line, the datestamp is only rendered when clicking the settings button as expected.

simonwep commented 4 years ago

@bjarnef That's perfect and definitely a (serious and weird) bug, I'll take a look into it!

simonwep commented 4 years ago

Bug found here and fixed here - I totally forgot about event bubbling. Could you try out the current bundles on master? If the fix works for you too I'll release a patch version!

bjarnef commented 4 years ago

Great! Thanks @Simonwep I just tried latest scripts bundle from here https://raw.githubusercontent.com/Simonwep/pickr/master/dist/pickr.min.js and it seems to fix the issue. It no longer trigger the click event on the ancestor element 👍 🥳 🎉

simonwep commented 4 years ago

Great! I've published 1.7.4 which comes with this fix :)

bjarnef commented 4 years ago

@Simonwep is something missing on npm since it shows the latest version is 1.7.2? https://www.npmjs.com/package/@simonwep/pickr

simonwep commented 4 years ago

@bjarnef Ay forgot to actually publish it - it's getting late here, sorry 😅