plotly.js
plotly.js copied to clipboard
Interactive portions of plot do not render correctly in a child window
When using Plotly to render in a child window, the hover functionality for plots seems to break completely. Any enabled buttons will render in the incorrect location, but seem to function correctly.
This CodePen shows the issue.
The attached screenshot demonstrates the issue as well.
I have reproduced in the latest stable Firefox and Chrome.
Thanks for report.
Interesting. I'm not sure what could cause that.
Everything renders correctly if you copy the injected styles from the parent window to the child. See this CodePen for a demonstration.
Things break if you try to interact with the plot by clicking on a data point or by zooming by clicking and selecting a zoom area.
@nielsenb-jf Interesting. Thanks for letting us know.
I have a branch in my fork that demonstrates fixing interactivity when rendering to a child window. Basically, it renders the drag cover to the document of the child window, instead of always to the parent window where Plotly was instantiated.
Looking at the code more, Plotly makes lots of assumptions about which document / window to render to. I'm not sure if it would be worth a pull request for only fixing this one issue, I'm sure there are others I haven't found yet.
Would there be interest in trying to make Plotly more window / document agnostic? It does potentially enable some interesting multi-window webapps with Plotly. This is a big enough undertaking that I'd rather not start it if there's no upstream interest in incorporating it.
If yes, what should be done about the issue of injecting stylesheets? Should they be injected to the document containing the original element given to Plotly? Is that even feasible?
What about getting the correct parent window? document.defaultView would be the 'modern' way to do it, but would limit support of IE versions before 9. Is that an issue? I've also found a polyfill of sorts on StackOverflow.
@nielsenb-jf thanks so much for your efforts.
that demonstrates fixing interactivity when rendering to a child window. Basically, it renders the drag cover to the document of the child window, instead of always to the parent window where Plotly was instantiated.
Fantastic!
Would there be interest in trying to make Plotly more window / document agnostic? It does potentially enable some interesting multi-window webapps with Plotly.
Yes. There is interest. But, this is not a priority by any means (explaining why no plotly team member has looked at this issue here in details).
what should be done about the issue of injecting stylesheets? Should they be injected to the document containing the original element given to Plotly? Is that even feasible?
I'm not exactly sure how to go about this (I personally don't know much about child windows). We currently inject our stylesheets using Lib.addStyleRule
here. Maybe there's a way to make it work on child windows? Alternatively, we could perhaps expose a method on Plotly
design to correctly populate child windows and have users that want to use plotly.js in child windows call that method upon window.open
.
but would limit support of IE versions before 9. Is that an issue?
We don't officially support IE < 9 at the moment, so no, this is not an issue.
I renamed my branch to 'child_window_fixes' to better describe the goal.
I tried testing as many plot types as I could, specifically looking for ones that make assumptions about the current window or document. The only other one that showed issues that I found were ones using range slider / selectors. The issue was the same as for other plots, the component actually managing the selection events was being attached to the parent, instead of to the child.
I also corrected an issue with restyle on plots in a child window not correctly reflecting the child window size.
So the question gets to be, should we clear up as many assumptions as possible, even if they don't cause issues? Or should things that are okay with happening in the parent window stay the way they are now? For example, the 'tester' element is currently appended to the parent window, not the child. This doesn't seem to be an issue, and I could see arguments for either way being "correct".
Yes. There is interest. But, this is not a priority by any means (explaining why no plotly team member has looked at this issue here in details).
I completely understand its not a priority for the team. It doesn't seem to be a priority for other users either since I seem to be the first person to stumble on it. It just seems to me that since Plotly takes a component as an argument, it shouldn't matter where that component is. Either way, I'm willing to try to make this work on my own with guidance when necessary.
I'm not exactly sure how to go about this (I personally don't know much about child windows). We currently inject our stylesheets using Lib.addStyleRule here.
I'm not either. I definitely need to look into this more. Any further advice you have would be greatly appreciated!
We don't officially support IE < 9 at the moment, so no, this is not an issue.
Good. nielsenb-jf/plotly.js@f10706dd45bc372239ae39184d4af2fc75375b49 wouldn't work with it.
Thanks again for your efforts!
Or should things that are okay with happening in the parent window stay the way they are now?
I would vote :+1: for this.
Plotly takes a component as an argument, it shouldn't matter where that component is.
Yes. No doubt about it.
So I guess this leave us to figure out what to do with our style sheets.
Perhaps we should move the Lib.addStyleRule
calls to the main Plotly.plot
routine (with a check to not replicate the rules in <style>
) so that every plotly.js graph is guaranteed to be accompanied with the correct style sheets.
@nielsenb-jf Would you be interested in trying that out?
New branch called child_css that incorporates the previous changes, as well as injecting styles in Plotly.plot
. Everything I've tested seems to work.
I changed pull_css
to generate a plotcss.js
file that does nothing but export the style rules. Those rules are then injected by the new plotcss_injector
. The injector will not inject a rule if there is already a matching rule in one of the target document's stylesheets. This prevents redundant rules when plot
is called multiple times on a document.
addStyleRule
had to be modified to not keep a reference to the stylesheet being modified. Otherwise, the wrong stylesheet would be modified when plot
is called first on the parent window, then on a child, or vice versa. If the document already has stylesheets, it defaults to adding the rule to the first one. I'm not sure if that's the best idea or not, but the only other idea I could come up with is to always modify the last stylesheet, assuming it was the one injected.
@nielsenb-jf great! Looks like you've got all the pieces together.
Here's a few recommendations before submitting your PR:
- Make
injectStyles
expect the graph divgd
as argument - Move the logic in
Lib.addStyleRule
toinjectStyle
(that way we won't have to check for<style>
for each style rule) and removeaddStyleRule
fromLib
. - We should only loop through all present style rules on the first
Plotly.plot
for a givengd
. So after that's done we should store an_
variable (e.g.gd._styleSheetLoaded = false
and thentrue
) to bypass that loop on subsequentPlotly.plot
calls. - make sure
npm run lint
passes - find a way to test that
injectStyles
works as expected viaPlotly.plot
in this suite. - maybe add a fallback for
ownerDocument
? MDN has no info about it in Safari. For example, we could early on inPlotly.plot
have:
gd._document = gd.ownerDocument.defaultView || window.document;
// and then use gd._document afterwards in the plotting code
I've got everything but the tests put together in the 'child_window_fixes' branch. Remaining work will happen there.
I've added some tests to the child_window_fixes branch.
Are we sure we want the tests in the plot_interact_test
suite? It feels a little cleaner to me to have them be their own thing. I could also see them in with the plot_api_test
suite since the CSS gets injected in the plot
method, which is defined in plot_api
. I'll defer to your judgement, it's not like we can't cut/paste the code somewhere else later.
Additionally, notice the buildFullSelector
function. It appears both Firefox and Chrome do some processing to CSS selectors (and the rules themselves if you dig into it some) during an insertRule
call. The result is equivalent, but it makes it difficult to test rules are inserted. I've done my best to make buildFullSelector
format the selector the way Firefox and Chrome seem to, but I can't find any documentation anywhere that guarantees that will stay consistent. If I can find some documentation, I'll add some tests. Note that buildFullSelector
gets used in the tests to check all selectors in plotcss
were inserted properly, but due to the aforementioned processing of rules, there is no testing to guarantee the rule itself is actually correct.
@nielsenb-jf Thanks so much!
Are we sure we want the tests in the plot_interact_test suite?
I agree. Your new test cases would look a lot better in a suite of their own. I'd call it plot_css_test.js
, but I'll leave it up to you to decide the file name.
Additionally, notice the buildFullSelector function. It appears both Firefox and Chrome do some processing to CSS selectors (and the rules themselves if you dig into it some) during an insertRule call.
Your new css/helpers.js
module looks great. Nice and DRY.
I think you're ready to make a PR to the main repo.
Squash those lint/formatting commits and PR away :rocket:
I fixed an issue with notifiers not appearing in the correct window, and with the _document
and _plotCSSLoaded
properties not being cleaned up on purge.
Assuming you don't see anything upsetting in those last commits, I'll squash up a PR.
Looks great. Sqaush away!
So git is being snotty about squashing a couple of the formatting commits due to some of the merging back and fourth I did (whoops). Also, I did a git rebase after opening the pull request...
Anyway, as per the contribution guide, the pull request to master on my fork.
@nielsenb-jf Great. Now please make a PR to this (main) repo. Thank you very much,
Looks likes a new plan of attack is needed (#826, #822). I put some thoughts on #829.
Another option would just be to update the documentation to say the element passed on plot
must be in the document Plotly is loaded in... :smile:
got same kind of problemas w the modebar and the buttons, got a screenshot from safari and chrome, same webpage
safari:
chrome:
Recently the issue #1360 I started has been added here and been closed.
I think I might have found the bug that caused my issue, and I have commented there what I think it could be so you can track what could be that produces this.
Hi. I've been developing a polymer 2.0 app, and now want to include plotly. However, because of the issue originally reported in #1433 I'm unable to do this without shimming the DOM with the shadyDOM from polymer 1. Has there any work towards resolving these issues? Thanks!
This issue has been tagged with NEEDS SPON$OR
A community PR for this feature would certainly be welcome, but our experience is deeper features like this are difficult to complete without the Plotly maintainers leading the effort.
Sponsorship range: $10k-$15k
What Sponsorship includes:
- Completion of this feature to the Sponsor's satisfaction, in a manner coherent with the rest of the Plotly.js library and API
- Tests for this feature
- Long-term support (continued support of this feature in the latest version of Plotly.js)
- Documentation at plotly.com/javascript
- Possibility of integrating this feature with Plotly Graphing Libraries (Python, R, F#, Julia, MATLAB, etc)
- Possibility of integrating this feature with Dash
- Feature announcement on community.plotly.com with shout out to Sponsor (or can remain anonymous)
- Gratification of advancing the world's most downloaded, interactive scientific graphing libraries (>50M downloads across supported languages)
Please include the link to this issue when contacting us to discuss.
FWIW, here's the latest Plotly SASS compiled into a CSS module that can be imported into a LitElement component. Anyone using LitElement and Plotly.js can copy this to into a file in their project and import it using the "Sharing styles" docs.
- doc: https://lit.dev/docs/components/styles/#sharing-styles
- plotly-styles.js: https://gist.github.com/rjsteinert/751901de6aca5804b1024c6737e9dc97
Much thanks to @darkengines for his suggestion here.
Lastly, here's a LitElement example of using Plotly.js. Placing the call to Plotly in the updated callback is key to making sure that the #myDiv is present in the shadowRoot of the component.
https://gist.github.com/rjsteinert/6ab3728643a15058c37a2502d59959e2
Seems unlikely that Plotly will ever be very useful as browsers and w3c standards progress, plotly's assumptions about its environment are increasingly unsustainable. Rather sad since some effort has been made to support browser ECMA module use.