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

Black output on Safari #18

Closed
m4mbart opened this issue Nov 29, 2017 · 11 comments
Assignees
Labels
bug

Comments

@m4mbart
Copy link

@m4mbart m4mbart commented Nov 29, 2017

Hi,
The color picker library works great on all browsers, except Safari. Actually it is also fully functional. I can interact with it, feed it color hex and receive color updates from the library.
Except the color wheel and lightness bar are all black. What could be wrong?

screen shot 2017-11-29 at 09 58 55

@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Nov 29, 2017

Hi, sorry to hear that you've been having problems with iro.js

I've just tested the library using Safari 11 (on both macOS and iOS) and can't seem to replicate the issue myself, so to get to the bottom of this I need to ask a couple of things:

  • Which Safari version are you having issues with? Also is it the mobile or desktop variant?
  • Which version of iro.js are you using? (As of 3.0.0, you can get the version number from iro.version)
  • Can you confirm that the landing page for the library demonstrates the same issue in the same browser?

Thanks! Hopefully we can figure out what's going on soon :)

@jaames jaames self-assigned this Nov 29, 2017
@jaames jaames added the bug label Nov 29, 2017
@m4mbart

This comment has been minimized.

Copy link
Author

@m4mbart m4mbart commented Nov 29, 2017

Hi, a little more info:

  • Safari version is 11.0.1
  • iro.js: 3.1.0
  • Hmm indeed, works fine on the landing page...

This is the init code:

this.colorWheel = new iro.ColorPicker("#colorWheel", {
          width: 240,
          height: 240,
          markerRadius: 6,
          color: this.selectedColor,
          borderWidth: 2,
          padding: 2,
          sliderHeight: 10,
          sliderWidth: 200,
          anticlockwise: true
        });
@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Nov 29, 2017

Hm, that's pretty strange.

There doesn't seem to be anything unusual about the options you're passing into iro.ColorPicker, although I wonder if the root of the problem is that iro.js is incorrectly parsing the value of this.selectedColor? Perhaps you could try swapping that with a simple hex color string so we can rule that out.

If that doesn't solve anything then I'd really appreciate it if you could reproduce the issue in a codepen / jsbin / etc :>

@m4mbart

This comment has been minimized.

Copy link
Author

@m4mbart m4mbart commented Nov 30, 2017

Yes I've tried that as well.. But without result.
Hmm I guess that will be a bit difficult, because it is deeply nested in a large Angular 4 application.

Could it have anything to do with the fact that it is placed inside an absolute positioned modal element? The difference between Safari and other browsers is interesting in this case...

@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Nov 30, 2017

Ah! I think the problem is that Angular might actually be "breaking" the SVG gradient URLs. This Stack Overflow answer explains the issue in depth (even though it's about filters, they work in a similar way to gradients).

TL;DR Angular adds a base tag to the head of your HTML document. The base tag changes the location of relative URLs on the page, so the color picker's UI elements can't find the gradients they're supposed to be using.

Who knows why that's a Safari-only issue, but it seems to be the most likely culprit here. Perhaps you could confirm?

I'll look around for possible solutions and hopefully get it fixed by tomorrow. :>

@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Nov 30, 2017

Small update: I've managed to reproduce the issue within Safari, and indeed, it appears that using a <base> tag may be the root of the problem.

I've been working on a fix over in the bug/broken-gradient-url branch. If you want to try it out, the compiled scripts are in the dist folder. :>

With that said, I noticed a second Safari-only issue. If you're using iro.js as part of a web application that implements client-side routing with the HTML history API, then the SVG gradients will also break when the user navigates to another view. I'm not quite sure how to solve that one to be honest, but I'll see what I can do.

@m4mbart

This comment has been minimized.

Copy link
Author

@m4mbart m4mbart commented Dec 1, 2017

Ah good to see that you're on to something here :) I'll test it right away...

@m4mbart

This comment has been minimized.

Copy link
Author

@m4mbart m4mbart commented Dec 1, 2017

Works like a charm! Even after navigating. Thanks a lot! :)

@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Dec 1, 2017

Great! I'm glad that fixed it :>

I pushed a couple of extra changes since my last comment:

  • Absolute gradient URLs are now only used in Safari.
  • A (somewhat hacky) solution for router navigation issues in case you do come across any: just call emit("history:stateChange") on the color picker instance whenever the route changes.

I'll push them into master tomorrow unless you come across any other related bugs \ o /

By the way, since you're using the iro.js in a large application, please don't hesitate to also submit feature requests/suggestions if you think there's anything that would be helpful!

@m4mbart

This comment has been minimized.

Copy link
Author

@m4mbart m4mbart commented Dec 1, 2017

Using commit 07efb22 now (latest as of this moment) and still working fine. I do not need to use emit("history:stateChange"). But that might be because I use the colorpicker inside a modal window that gets reconstructed after re-routing re-opening.

@jaames

This comment has been minimized.

Copy link
Owner

@jaames jaames commented Dec 2, 2017

Changes merged into master, so I'm going to close this thread now.
Thanks again for the bug report, this was a pretty tricky one. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.