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

Broken on Safari 14 #246

Open
swaterfall opened this issue Sep 23, 2020 · 22 comments
Open

Broken on Safari 14 #246

swaterfall opened this issue Sep 23, 2020 · 22 comments
Labels
browser quirk Deprecation, browser idiosyncrasies etc... help wanted Extra attention is needed

Comments

@swaterfall
Copy link

What is the current behavior?

Save button doesn't function correctly and Button color not updating to reflect selection.

Please provide the steps to reproduce:

Use the demo https://simonwep.github.io/pickr/ on the latest version of Safari.

Your environment:

Version (see Pickr.version): 1.7.2
Used bundle (es5 or normal one): Both
Used theme (default is classic): All
Browser-version:  Safari 14.0 (15610.1.28.1.9, 15610)
Operating-system:  macOS 10.15.6 (19G2021)
@swaterfall swaterfall added the bug Something isn't working label Sep 23, 2020
@oliverwaters
Copy link

I'm also experiencing this on the latest version of Safari

@simonwep
Copy link
Owner

I don't own a mac nor have access to the latest Safari version (only v12) - You one of you describe what is happening in detail? Maybe there's an error message? Or is there a page where I can see all the changes from Safari 13 to Safari 14?

@swaterfall
Copy link
Author

There are no errors, it appears the actual values work fine the issue is actually rendering. If I resize the browser window it shows the correct colours:

https://www.dropbox.com/s/45zzf88gcq68k7c/picker-safari-issue-2.mov?dl=0

Interestingly the Classic theme selection preview on the left hand side works fine:

Screenshot 2020-09-24 at 10 48 36

but it doesn't on Monolith:

Screenshot 2020-09-24 at 10 48 50

Clicking save on both only updates the button colour after resizing the browser (and other such interactions)

@simonwep
Copy link
Owner

Ugh, that is really strange (and difficult to fix without safari) - at least I won't be able to fix that, I'll mark this issue maybe someone else could take a look at it :)

@simonwep simonwep added help wanted Extra attention is needed browser quirk Deprecation, browser idiosyncrasies etc... labels Sep 24, 2020
@yahoo0742
Copy link

It works well on Safari version 13.1.2 (13609.3.5.1.5) BTW.

@MrMooky
Copy link

MrMooky commented Oct 26, 2020

That is definitely a Safari related bug. I was able to at least update the color after saving with this method:

pickr.on('save', function(hsva, instance){
    pickr.hide();
    $input.focus();
});

But the problem still exists even on page load. The correct "default" color is not applied until the browser is either resized, or even when I toggle the console. I even tried $(window).trigger('resize') after initialising, but without luck. It even works if I open it in Safari, open another window (any other app) and then return to Safari. Really strange.

@fjaxyu
Copy link

fjaxyu commented Oct 27, 2020

I had this same issue. For me, the color text output in the bottom left was updating as I changed the slider, but the color block on the left wasn't. Same thing would happen when I clicked, sometimes, on two similar colors in the swatches.

When the update occurred, I looked and it seemed to be setting the active-color block's color style instead of the background-color for some reason.

I set an event listener:

pickr.on('change', pickrColorChange);

When the color change event happened, I'd grab the active color element and set the background color instead of the color:

function pickrColorChange(instance) {
    const rgbaColorArray = instance.toRGBA();
    var currentColorDisplayElement = document.querySelector('.pcr-current-color');
    currentColorDisplayElement.style.backgroundColor = `rgb(${rgbaColorArray[0]}, ${rgbaColorArray[1]}, ${rgbaColorArray[2]})`;
}

I didn't need opacity (but that's an easy addition) and only had one instance ever, so this worked for my needs. You could model your solution around this if you need.

If it's setting the color style, maybe the solution here is to change the style being set from "color" to "background-color" in the color change method in Pickr? or both?

@edwinfinch
Copy link

Thanks @fjaxyu for the inspiration. For anyone else that has multiple picker instances, here's how I fixed it with just two lines:

let button = instance.getRoot().button;
button.style.backgroundColor = button.style.color;

I placed this in the save event handler, but you should be able to use this anywhere that you have access to the Pickr instance.

@simonwep simonwep removed the bug Something isn't working label Nov 15, 2020
@MrMooky
Copy link

MrMooky commented Nov 16, 2020

@edwinfinch This works with all colors except #ffffff. I had to put this in a init method because the problem occurs directly on page load for me.

.on('init', function(instance){
    var button = instance.getRoot().button;
    button.style.backgroundColor = instance._color.toHEXA().toString();
    // button.style.backgroundColor = button.style.color;
});

Screenshot 2020-11-16 at 08 22 07

I tried a few things that came to mind, but was not able to get white working. Does it work for you with #ffffff?

@edwinfinch
Copy link

@MrMooky it does work for me so I won't be able to help you unfortunately.

@simonwep why did you remove the bug label?

@simonwep
Copy link
Owner

@edwinfinch I'd rather say that this is a safari-related bug where currentColor does not update when color should trigger a re-paint. It wouldn't be the first time Safari doesn't follow a spec or behaves in a weird way hence I would only call that a "browser-quirk", it's not a general bug. Changing color should change currentColor and therefore background. It's part of the spec:

The value of the ‘color’ property. The used value of the ‘currentColor’ keyword is the computed value of the ‘color’ property. If the ‘currentColor’ keyword is set on the ‘color’ property itself, it is treated as ‘color: inherit’.

Safari seems to ignore the computed part. What I found out with a friends laptop is that, if you resize the browser, the color does change. So Safari is missing a repaint cycle.

A fix would be using CSS variables, I'll take a look at this on the next weekend.

@caspii
Copy link

caspii commented Apr 1, 2021

Still broken on Safari 14.0.3 (16610.4.3.1.7)

@simonwep
Copy link
Owner

simonwep commented Apr 2, 2021

@MrMooky @swaterfall @caspii @edwinfinch

I just pushed an updated version with CSS Variables instead. Of course this means that IE11 support is completely gone but I never intented to support it anyways (and anyways, safari seems to be the new IE11). Since I don't have any device with safari on it, could you please verify if it's now working as expected?

@Teraskull
Copy link

Teraskull commented Apr 3, 2021

@simonwep I updated to this version and now color palettes are not visible. They still function, but are not previewed.

This is on Windows 10.

image

@simonwep
Copy link
Owner

simonwep commented Apr 3, 2021

@Teraskull Huh, there are many browsers for windows 10, which one are you using?

@Teraskull
Copy link

Edge Chromium Version 91.0.838.3 (Official build) dev (64-bit)

@simonwep
Copy link
Owner

simonwep commented Apr 3, 2021

Okay, it works on the stable version (89) so I don't consider this as a bug (this one is on edge and you're using a dev version, it's very likely that some things don't work there). After somebody tested it with a safari v13+ browser I'll release a new patch version and close this one.

@Teraskull
Copy link

Teraskull commented Apr 3, 2021

@simonwep My bad, I forgot to update the theme file as well, sorry for distraction :)

@caspii
Copy link

caspii commented Apr 7, 2021

@simonwep thanks for your efforts!

Here's what I'm seeing on Version 14.0.3 (16610.4.3.1.7)

image

@simonwep
Copy link
Owner

simonwep commented Apr 7, 2021

@caspii would you be able to figure out what's wrong with the swatches and color-preview? I can imagine it'd take me quite some time to figure it out without being able to test it...

@caspii
Copy link

caspii commented Apr 7, 2021

@simonwep unfortunately I'm not a frontend-crack, but here's what I've found out:

  • The problem also exists on Chrome (Version 89.0.4389.114 (Official Build) (arm64))
  • A CSS variable color is being applied to the pcr-button button, but it's not working. Even overwriting this property in dev tools with a normal (non-variable) does not cause the color to be applied to the button.
  • I can see the following problem in dev tools which may be related:

image

@max-sixblade
Copy link

Yes, unfortunately broken on safari 14 both desktop and mobile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser quirk Deprecation, browser idiosyncrasies etc... help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

10 participants