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 correct parent document when necessary: fixes 702 #2

Open
wants to merge 141 commits into
base: master
Choose a base branch
from

Conversation

nielsenb-jf
Copy link
Owner

This pull request fixes the issues noted in issue 702. Essentially, if Plotly was provided an element in a different window or tab, the plot would not be styled correctly and interactivity (hovers, drag zoom, etc.) would not work.

The following are the big changes to fix this issue:

  • Graph divs now keep a reference to their 'parent' document in the `_document' property.
  • Styles are now injected to the parent document on the first call to Plotly.plot.
  • Anything that draws over a document for interactivity purposes (eg. dragcovers), now uses the _document reference to ensure it is drawn in the correct document.
  • Notifiers now require a graph div as the first argument so that notifications are rendered in the correct place

etpinard and others added 30 commits June 27, 2016 17:54
  - to determine if an attribute val is valid
 - now only coerce numbers, dates and boolean to string
- to determine whether user 'data' and 'layout'
  are valid in plotly.js
- only value that have same correct length and all
  item valid are consisdered valid.
This fixes the issue with the dragcover not being appended to the
correct child when the plot is rendered on a window other than the
parent.
- make Plotly.validate return a 1-level deep array of
  error objects
- add 'container' and 'trace' key in each error object
  to locate the error in data trace or layout.
- add 'astr' to error object (for easy plug into restyle and relayout)
This fixes the issue with range slider events being tied to the parent
window, as opposed to the child, when rendering to a div in a child
window.
Introducing Plotly.validate(data, layout)
This fixes the assumption that the plot should be sized based on the
window Plotly was instantiated in, instead of the window Plotly is
drawing the plot in.
The previous behavior was to inject CSS into the document the Plotly JS
is loaded in. This causes plots rendered in a different document to not
be styled correctly. Now, the CSS is injected into the correct document
during Plotly.plot.
files.

Also change the built plotcss file to export the rules, not inject them.
Add a z-index to js-plotly-tester as a workaround for a chrome bug
Fix repeated val case in scattermapbox convert step
nielsenb-jf and others added 27 commits July 20, 2016 10:48
This fixes the issue with notifications not appearing in the correct
window when plotting to a child window. The notification is now appended
to the body of the document the graph div is a child of.
This fixes issue where styles are not properly applied to a child window
if the first plot is in the parent, or vice versa.
The _plotCSSLoaded property is set to true after injecting the CSS.
…ed in.

This reference is used where necessary to ensure functionality when
plotting to a window other than the one Plotly was instantiated in.
'inserts styles on initial plot' - Deletes all plotly CSS in the
document (in case other tests have run previously), does a plot, and
verifies the CSS has been injected.

'inserts styles in a child window document' - Creates a plot in a child
window, and makes sure the CSS has been injected. The child window is
then closed, no cleanup required.

'does not insert duplicate styles' - Deletes all plotly CSS in the
document, plots to the graph div twice, and ensures the required Plotly
CSS only occurs once.
This fixes the issue with notifications not appearing in the correct
window when plotting to a child window. The notification is now appended
to the body of the document the graph div is a child of.
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

Successfully merging this pull request may close these issues.

6 participants