-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Interactive portions of plot do not render correctly in a child window #702
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
Comments
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.
Fantastic!
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'm not exactly sure how to go about this (I personally don't know much about child windows). We currently inject our stylesheets using
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".
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 either. I definitely need to look into this more. Any further advice you have would be greatly appreciated!
Good. nielsenb-jf/plotly.js@f10706d wouldn't work with it. |
Thanks again for your efforts!
I would vote 👍 for this.
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 @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 I changed
|
@nielsenb-jf great! Looks like you've got all the pieces together. Here's a few recommendations before submitting your PR:
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 Additionally, notice the |
@nielsenb-jf Thanks so much!
I agree. Your new test cases would look a lot better in a suite of their own. I'd call it
Your new I think you're ready to make a PR to the main repo. Squash those lint/formatting commits and PR away 🚀 |
I fixed an issue with notifiers not appearing in the correct window, and with the 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, |
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 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:
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. |
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. |
Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson |
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.
The text was updated successfully, but these errors were encountered: