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

use unique ids #67

Closed
timelyportfolio opened this issue Jul 27, 2016 · 10 comments
Closed

use unique ids #67

timelyportfolio opened this issue Jul 27, 2016 · 10 comments

Comments

@timelyportfolio
Copy link
Contributor

timelyportfolio commented Jul 27, 2016

When using mutliple svg from svglite in the browser, the id for clippath and other elements will very likely conflict. Would it be possible to use uuid to prevent this conflict?

@hadley
Copy link
Member

hadley commented Aug 15, 2016

Please provide a reproducible example of the problem.

@timelyportfolio
Copy link
Contributor Author

Sure, if you open this in Chrome (seems to be the most reliable in showing the bug), you will see the clipPath from the first plot is applied to the second, since the DOM requires unique selector id. There is a #cp1 and #cp2 for both plots, and the cp1 and cp2 are applied to the second plot.

library(svglite)
library(htmltools)

browsable(
  tagList(
    htmlSVG({plot(1:10,pch=10)},height=4,width=7),
    htmlSVG({contour(volcano)},height=20,width=20)
  )
)

image

image

Deleting the cp2 from the first corrects the problem.

svglite_nonuniqueid

@lionel-
Copy link
Member

lionel- commented Oct 20, 2016

@timelyportfolio I don't see uuid in the SVG spec. What do you mean?

@hadley generating random ids seems like it would create difficulties regarding reproducibility. I think it would be better to add an argument id_prefix = "" to all device functions, what do you think?

@hadley
Copy link
Member

hadley commented Oct 20, 2016

@lionel- alternatively maybe we could use a simple hashing algorithm to ensure that the id is unique? There's only four values, so it shouldn't be too hard

@timelyportfolio
Copy link
Contributor Author

uuid is not in the SVG spec. Here is the Wiki definition. For reference, here is how it is handled in htmlwidgets ramnathv/htmlwidgets#213.

@lionel-
Copy link
Member

lionel- commented Oct 20, 2016

@timelyportfolio thanks!

@hadley would using boost::hash() on a std::pair() of std::pair() be a reasonable approach?

@hadley
Copy link
Member

hadley commented Oct 20, 2016

Sure, although it might be just as easy to code up something by hand, e.g. paste together the hex representation of each value (maybe coerce to float to make shorter?)

lionel- added a commit to lionel-/svglite that referenced this issue Oct 21, 2016
@timelyportfolio
Copy link
Contributor Author

timelyportfolio commented Oct 24, 2016

Unfortunately, this solution does not rectify the situation. Using the same reproducible example as before, I get the same clippaths for each svg. Am I missing something?

example code

library(svglite)
library(htmltools)

browsable(
    tagList(
        htmlSVG({plot(1:10,pch=10)},height=4,width=7),
        htmlSVG({contour(volcano)},height=20,width=20)
    )
)

replicated clippath ids

image

@lionel-
Copy link
Member

lionel- commented Oct 24, 2016

yes there was a bug but already fixed, soon to be on master ;)

@timelyportfolio
Copy link
Contributor Author

Great thanks!

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