Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

dcc.Graph: Improve responsiveness, expose resize status, allow wrapper extension #706

Merged
merged 53 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
6a4a2b3
wip - responsive graph
Nov 20, 2019
d721038
render and layout strategies
Nov 21, 2019
6361f5b
unified render, overrideable responsive behavior
Nov 21, 2019
a02a986
cleanup
Nov 21, 2019
2f957bf
Merge branch 'dev' into 605-graph-resize
Marc-Andre-Rivet Nov 21, 2019
33afc15
undo webpack changes
Nov 21, 2019
260c90b
Merge branch '605-graph-resize' of github.com:plotly/dash-core-compon…
Nov 21, 2019
ed02f5e
document `responsive`
Nov 21, 2019
c2fcd2c
format
Nov 21, 2019
3badeb9
comments
Nov 21, 2019
327a9e3
format
Nov 21, 2019
46aed75
clean up overrides
Nov 21, 2019
c3f6fd9
rework
Nov 29, 2019
0b78f42
typo
Nov 29, 2019
b36919d
Merge branch 'dev' into 605-graph-resize
Marc-Andre-Rivet Nov 29, 2019
b784df8
rework layout logic
Nov 29, 2019
dc6b4ae
- all graph responsiveness combos
Nov 29, 2019
de29b48
is True/False
Nov 29, 2019
9ba52e6
- default to responsive 'auto'
Nov 29, 2019
41527bf
update prop description
Nov 29, 2019
f0ba158
resize counter
Dec 2, 2019
79d3bfa
update tests
Dec 2, 2019
0d81609
changelog
Dec 2, 2019
860d133
update responsive docs
Dec 2, 2019
4555c68
lint
Dec 2, 2019
c8a5679
responsive relation with style
Dec 3, 2019
c01eb32
- private layout/config transform methods to customize plot behavior
Dec 5, 2019
28e9dd6
update tests
Dec 5, 2019
c6a2d4a
- remove counters (requires plotly.js 4392 to fully work)
Dec 5, 2019
577cc8b
div -> fragment wrapper
Dec 6, 2019
571db5a
lint
Dec 6, 2019
a30abcb
plot on transform functions update
Dec 10, 2019
71852a3
format
Dec 10, 2019
3e4de1c
use props arg
Dec 10, 2019
c60b59c
reduce meddling to a minimum when using `auto` mode - eval responsive…
Dec 10, 2019
ed1b823
update responsiveness and tests based on plotly.js' behavior (auto) w…
Dec 10, 2019
cd41f75
Due to react-resize-detector, need to move back to the div wrapper to…
Dec 11, 2019
68b9c8e
dash-graph and 'legacy' js-plotly-plot class
Dec 11, 2019
a59682e
Update dash-graph-resizing to dash-graph--pending
Dec 11, 2019
8beae49
Remove `dash-graph--pending` from scope
Dec 12, 2019
99b9a08
add it back...
Dec 12, 2019
74b08fc
- clean up className passing
Dec 12, 2019
fb218a5
- add pending class on `Plotly.react`
Dec 12, 2019
8addc80
Merge branch 'dev' into 605-graph-resize
Marc-Andre-Rivet Dec 12, 2019
d1a1dd5
update changelog
Dec 12, 2019
701809b
Merge branch '605-graph-resize' of github.com:plotly/dash-core-compon…
Dec 12, 2019
530ef65
Merge branch 'dev' into 605-graph-resize
Marc-Andre-Rivet Dec 13, 2019
cb77c3b
- update to plotly.js 1.51.3
Dec 16, 2019
b39406e
re-remove js-plotly-plot
Dec 16, 2019
56d883b
Clean up props usage
Dec 16, 2019
6867be2
clean up the clean up
Dec 16, 2019
c744ae9
clean up the clean up of the clean up
Dec 16, 2019
de59930
remove useless gitignore entry
Dec 16, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"prettier": "^1.14.2",
"react": "^16.8.6",
"react-dom": "^16.8.6",
"react-resize-detector": "^4.2.1",
"style-loader": "^0.23.1",
"styled-jsx": "^3.1.1",
"webpack": "^4.37.0",
Expand All @@ -91,4 +92,4 @@
"react": "^16.0.0",
"react-dom": "^16.0.0"
}
}
}
15 changes: 15 additions & 0 deletions src/components/Graph.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ PlotlyGraph.propTypes = {
*/
id: PropTypes.string,

/**
* If True, the Plotly.js plot will be fully responsive to window resize
* and parent element resize event. This is achieved by overriding
* `config.responsive` to True, `figure.layout.autosize` to True and unsetting
* `figure.layout.height` and `figure.layout.width`.
* If False (default), the Plotly.js plot may exhibit certain responsive
* behaviors on its own but nothing is done in Dash to help this behavior.
* This is the legacy behavior of the Graph component.
* If 'auto', the Graph will determine if the Plotly.js plot can be made fully
* responsive (True) or not (False) based on the values in `config.responsive`,
* `figure.layout.autosize`, `figure.layout.height`, `figure.layout.width`.
*/
responsive: PropTypes.oneOf([true, false, 'auto']),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow users to define the behavior they want for their graph, allowing them to override the normal behavior for their config/figure values.


/**
* Data from latest click event. Read-only.
*/
Expand Down Expand Up @@ -491,6 +505,7 @@ PlotlyGraph.defaultProps = {
layout: {},
frames: [],
},
responsive: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaults to responsive False which is the current behavior.

animate: false,
animation_options: {
frame: {
Expand Down
105 changes: 93 additions & 12 deletions src/fragments/Graph.react.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,42 @@
import React, {Component} from 'react';
import {clone, equals, filter, has, includes, isNil, omit, type} from 'ramda';
import ResizeDetector from 'react-resize-detector';
import {
clone,
equals,
filter,
has,
includes,
isNil,
mergeDeepRight,
omit,
type,
} from 'ramda';
import PropTypes from 'prop-types';
import {graphPropTypes, graphDefaultProps} from '../components/Graph.react';
/* global Plotly:true */

/**
* `autosize: true` causes Plotly.js to conform to the parent element size.
* This is necessary for `dcc.Graph` call to `Plotly.Plots.resize(target)` to do something.
*
* Users can override this value for specific use-cases by explicitly passing `autoresize: true`
* if `responsive` is not set to True.
*/
const DEFAULT_LAYOUT = {
autosize: true,
};

/**
* `responsive: true` causes Plotly.js to resize the graph on `window.resize`.
* This is necessary for `dcc.Graph` call to `Plotly.Plots.resize(target)` to do something.
*
* Users can override this value for specific use-cases by explicitly passing `responsive: false`
* if `responsive` is not set to True.
*/
const DEFAULT_CONFIG = {
responsive: true,
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When responsive is true, whether through true or auto, will apply these values to config/figure to get the desired behavior.

const filterEventData = (gd, eventData, event) => {
let filteredEventData;
if (includes(event, ['click', 'hover', 'selected'])) {
Expand Down Expand Up @@ -79,10 +112,13 @@ class PlotlyGraph extends Component {
this._hasPlotted = false;
this._prevGd = null;
this.graphResize = this.graphResize.bind(this);
this.isResponsive = this.isResponsive.bind(this);
}

plot(props) {
const {figure, animate, animation_options, config} = props;
const responsive = this.isResponsive(props);

const gd = this.gd.current;

if (
Expand All @@ -92,11 +128,27 @@ class PlotlyGraph extends Component {
) {
return Plotly.animate(gd, figure, animation_options);
}

let layoutClone = figure.layout;
if (layoutClone) {
if (responsive) {
layoutClone = mergeDeepRight(figure.layout, DEFAULT_LAYOUT);
delete layoutClone.height;
delete layoutClone.width;
} else {
layoutClone = clone(figure.layout);
}
}

const configClone = responsive
? mergeDeepRight(config, DEFAULT_CONFIG)
: clone(config);

return Plotly.react(gd, {
data: figure.data,
layout: clone(figure.layout),
layout: layoutClone,
frames: figure.frames,
config: config,
config: configClone,
}).then(() => {
const gd = this.gd.current;

Expand Down Expand Up @@ -154,7 +206,28 @@ class PlotlyGraph extends Component {
clearExtendData();
}

isResponsive(props) {
const {config, figure, responsive} = props;

if (type(responsive) === 'Boolean') {
return responsive;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If responsive is a boolean, use as-is. Otherwise, determined auto responsiveness by looking at config/figure for the graph.


return (
(config.responsive || isNil(config.responsive)) &&
(!figure.layout ||
((figure.layout.autosize || isNil(figure.layout.autosize)) &&
(isNil(figure.layout.height) ||
isNil(figure.layout.width))))
);
}

graphResize() {
const responsive = this.isResponsive(this.props);
if (!responsive) {
return;
}

const gd = this.gd.current;
if (gd) {
Plotly.Plots.resize(gd);
Expand Down Expand Up @@ -221,10 +294,7 @@ class PlotlyGraph extends Component {
}

componentDidMount() {
this.plot(this.props).then(() => {
window.addEventListener('resize', this.graphResize);
});

this.plot(this.props);
if (this.props.extendData) {
this.extend(this.props);
}
Expand All @@ -238,7 +308,6 @@ class PlotlyGraph extends Component {
Plotly.purge(gd);
}
}
window.removeEventListener('resize', this.graphResize);
}

shouldComponentUpdate(nextProps) {
Expand Down Expand Up @@ -277,15 +346,27 @@ class PlotlyGraph extends Component {

return (
<div
key={id}
id={id}
ref={this.gd}
key={id}
data-dash-is-loading={
(loading_state && loading_state.is_loading) || undefined
}
style={style}
className={className}
/>
>
Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graph is now wrapped in a div that applies the styling -- the graph div itself has consistent styling. This appears to help responsiveness to work correctly in flex flows.

  • Need to make sure this is truly useful, not development side-effects.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still concerned about adding js-plotly-plot to the outer div - seems like having two nested divs with this class could lead to strange behavior, if it's actually getting used for something in CSS. Can you explain the reasoning here?

Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable. My goal here is for old versions of DDK to continue having the same behavior. Given the way DDK styling works and the stated goal of minimizing custom CSS, this should be fine. To break this behavior would require users to assign display: flex; to js-plotly-plot. The next DDK release will be using .dash-graph.

For the DCC world, this is probably a non-issue, the general idea is that we support classes & attributes with the dash- prefix, everything else is implementation details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't add js-plotly-plot, dash>=1.8.0 will be a breaking version.
If we do add it, we can always create an issue to remove it and tag that issue as Dash v2.x.

Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As pointed out by @alexcjohnson, in the end the legacy
js-plotly-plot is not necessary as old ddk uses the forked version of the graph anyway.

Discussing this with @wbrgss, it seems like the only scenario left is a new version of DDK pointing to an old version of Dash, but that can be handled through CSS with *:not(.dash-graph) > .js-plotly-js in DDK itself.

<ResizeDetector
handleHeight={true}
handleWidth={true}
refreshMode="debounce"
refreshOptions={{trailing: true}}
refreshRate={50}
onResize={this.graphResize}
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

element resize observer - either uses ResizeObserver in newer browsers, or polyfills with a DOM/css method if it doesn't. Debounce / trail / wait to prevent making useless calls.

<div
ref={this.gd}
style={{height: '100%', width: '100%'}}
className={className}
/>
</div>
);
}
}
Expand Down