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

Do not add <base> href to SVG clip paths during toImage #3272

Merged
merged 8 commits into from
Nov 26, 2018

Conversation

etpinard
Copy link
Contributor

fixes #3260

@plotly/plotly_js to do this "properly" i.e. w/o doing something hacky in Snapshot.toSVG like

d3.selectAll('[clipPath'].each(function() {
  var s = d3.select(this);
  var cp = s.attr('clipPath');
  s.attr('clipPath'. cp.replace(URL_REGEX, '');
})

I made Drawing.setClipUrl expect gd as third argument. This means we can stash the <base> href on gd._context (instead of on Drawing) and use it in setClipUrl.

- pass it as third argument, to keep setClipUrl
  compatible with `selection.call(method, ...args)`
- needed to pass gd to ErrorBars.plot to clip errorbars
... but I'm not sure that's correct. We might still need that
    baseUrl for staticPlot:true graph don't get exported
    via Plotly.toImage or Plotly.downloadImage.
... now that we pass 'gd' to Drawing.setClipUrl
... instead of 'staticPlot', which can conflict when
    users want to draw a 'static plot' in the browser.
src/plot_api/to_image.js Outdated Show resolved Hide resolved
@antoinerg
Copy link
Contributor

antoinerg commented Nov 20, 2018

Looks good to me 💃 but maybe let's wait for @Braintelligence to confirm it fixed his issue.

@alexcjohnson
Copy link
Collaborator

I'm happy now, and your update to @Braintelligence's fiddle #3260 (comment) works for me - so 💃 from my side but perhaps we can wait for @Braintelligence to confirm.

@Braintelligence
Copy link

Sorry guys, I am on vacation. I can confirm tomorrow, but if my fiddle doesn't behave like described anymore, it should be alright!

@etpinard etpinard merged commit 8ef1445 into master Nov 26, 2018
@etpinard etpinard deleted the snapshot-clippath-base branch November 26, 2018 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug?] Strange clip-path problem
4 participants