Skip to content

Commit

Permalink
Fix wrong Y-axis range for stacked bar chart (#5029)
Browse files Browse the repository at this point in the history
* #5026 Fix wrong Y-axis range for stacked bar chart

* Update tests

* Use Plotly's built-in algorinthm to compute Y-axis range

* Update tests

* Revert previous solution (yRange-related code)

* Revert other unrelated changes

* Revert other unrelated changes

* Move chart rendering to own file and ensure that rendering steps will occur in necessary order

* Reduce amount of plot updates by mergin separate updates into a sigle cumulative update

* Give better names for several functions
  • Loading branch information
kravets-levko committed Jul 17, 2020
1 parent 6c349ea commit a1255b4
Show file tree
Hide file tree
Showing 14 changed files with 284 additions and 164 deletions.
99 changes: 30 additions & 69 deletions viz-lib/src/visualizations/chart/Renderer/PlotlyChart.jsx
Original file line number Diff line number Diff line change
@@ -1,87 +1,48 @@
import { isArray, isString, isObject, startsWith } from "lodash";
import React, { useState, useEffect, useContext } from "react";
import React, { useState, useEffect, useContext, useRef } from "react";
import useMedia from "use-media";
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
import { RendererPropTypes } from "@/visualizations/prop-types";
import resizeObserver from "@/services/resizeObserver";
import { visualizationsSettings } from "@/visualizations/visualizationsSettings";
import getChartData from "../getChartData";
import { Plotly, prepareData, prepareLayout, updateData, updateLayout, applyLayoutFixes } from "../plotly";

function catchErrors(func, errorHandler) {
return (...args) => {
try {
return func(...args);
} catch (error) {
// This error happens only when chart width is 20px and looks that
// it's safe to just ignore it: 1px less or more and chart will get fixed.
if (isString(error) && startsWith(error, "ax.dtick error")) {
return;
}
errorHandler.handleError(error);
}
};
}
import initChart from "./initChart";

export default function PlotlyChart({ options, data }) {
const [container, setContainer] = useState(null);
const [chart, setChart] = useState(null);

const errorHandler = useContext(ErrorBoundaryContext);
const isMobile = useMedia({ maxWidth: 768 });
const errorHandlerRef = useRef();
errorHandlerRef.current = errorHandler;

useEffect(
catchErrors(() => {
if (container) {
const plotlyOptions = {
showLink: false,
displaylogo: false,
};
const isMobile = useMedia({ maxWidth: 768 });
const isMobileRef = useRef();
isMobileRef.current = isMobile;

if (visualizationsSettings.hidePlotlyModeBar) {
plotlyOptions.displayModeBar = false;
useEffect(() => {
if (container) {
let isDestroyed = false;

const chartData = getChartData(data.rows, options);
const _chart = initChart(container, options, chartData, visualizationsSettings, error => {
errorHandlerRef.current.handleError(error);
});
_chart.initialized.then(() => {
if (!isDestroyed) {
setChart(_chart);
}
});
return () => {
isDestroyed = true;
_chart.destroy();
};
}
}, [options, data, container]);

const chartData = getChartData(data.rows, options);
const plotlyData = prepareData(chartData, options);
const plotlyLayout = { ...prepareLayout(container, options, plotlyData), dragmode: !isMobile ? "zoom" : false };

// It will auto-purge previous graph
Plotly.newPlot(container, plotlyData, plotlyLayout, plotlyOptions).then(
catchErrors(() => {
applyLayoutFixes(container, plotlyLayout, options, (e, u) => Plotly.relayout(e, u));
}, errorHandler)
);

container.on(
"plotly_restyle",
catchErrors(updates => {
// This event is triggered if some plotly data/layout has changed.
// We need to catch only changes of traces visibility to update stacking
if (isArray(updates) && isObject(updates[0]) && updates[0].visible) {
updateData(plotlyData, options);
updateLayout(plotlyLayout, options, plotlyData);
Plotly.relayout(container, plotlyLayout);
}
}, errorHandler)
);

const unwatch = resizeObserver(
container,
catchErrors(() => {
applyLayoutFixes(container, plotlyLayout, options, (e, u) => Plotly.relayout(e, u));
}, errorHandler)
);
return unwatch;
}
}, errorHandler),
[options, data, container, isMobile]
);

// Cleanup when component destroyed
useEffect(() => {
if (container) {
return () => Plotly.purge(container);
if (chart) {
chart.setZoomEnabled(!isMobile);
}
}, [container]);
}, [chart, isMobile]);

return <div className="chart-visualization-container" ref={setContainer} />;
}
Expand Down
134 changes: 134 additions & 0 deletions viz-lib/src/visualizations/chart/Renderer/initChart.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { isArray, isObject, isString, isFunction, startsWith, reduce, merge, map, each } from "lodash";
import resizeObserver from "@/services/resizeObserver";
import { Plotly, prepareData, prepareLayout, updateData, updateYRanges, updateChartSize } from "../plotly";

function createErrorHandler(errorHandler) {
return error => {
// This error happens only when chart width is 20px and looks that
// it's safe to just ignore it: 1px less or more and chart will get fixed.
if (isString(error) && startsWith(error, "ax.dtick error")) {
return;
}
errorHandler(error);
};
}

// This utility is intended to reduce amount of plot updates when multiple Plotly.relayout
// calls needed in order to compute/update the plot.
// `.append()` method takes an array of two element: first one is a object with updates for layout,
// and second is an optional function that will be called when plot is updated. That function may
// return an array with same structure if further updates needed.
// `.process()` merges all updates into a single object and calls `Plotly.relayout()`. After that
// it calls all callbacks, collects their return values and does another loop if needed.
function initPlotUpdater() {
let actions = [];

const updater = {
append(action) {
if (isArray(action) && isObject(action[0])) {
actions.push(action);
}
return updater;
},
process(plotlyElement) {
if (actions.length > 0) {
const updates = reduce(actions, (updates, action) => merge(updates, action[0]), {});
const handlers = map(actions, action => (isFunction(action[1]) ? action[1] : () => null));
actions = [];
return Plotly.relayout(plotlyElement, updates).then(() => {
each(handlers, handler => updater.append(handler()));
return updater.process(plotlyElement);
});
} else {
return Promise.resolve();
}
},
};

return updater;
}

export default function initChart(container, options, data, additionalOptions, onError) {
const handleError = createErrorHandler(onError);

const plotlyOptions = {
showLink: false,
displaylogo: false,
};

if (additionalOptions.hidePlotlyModeBar) {
plotlyOptions.displayModeBar = false;
}

const plotlyData = prepareData(data, options);
const plotlyLayout = prepareLayout(container, options, plotlyData);

let isDestroyed = false;

let updater = initPlotUpdater();

function createSafeFunction(fn) {
return (...args) => {
if (!isDestroyed) {
try {
return fn(...args);
} catch (error) {
handleError(error);
}
}
};
}

let unwatchResize = () => {};

const promise = Promise.resolve()
.then(() => Plotly.newPlot(container, plotlyData, plotlyLayout, plotlyOptions))
.then(
createSafeFunction(() =>
updater
.append(updateYRanges(container, plotlyLayout, options))
.append(updateChartSize(container, plotlyLayout, options))
.process(container)
)
)
.then(
createSafeFunction(() => {
container.on(
"plotly_restyle",
createSafeFunction(updates => {
// This event is triggered if some plotly data/layout has changed.
// We need to catch only changes of traces visibility to update stacking
if (isArray(updates) && isObject(updates[0]) && updates[0].visible) {
updateData(plotlyData, options);
updater.append(updateYRanges(container, plotlyLayout, options)).process(container);
}
})
);

unwatchResize = resizeObserver(
container,
createSafeFunction(() => {
updater.append(updateChartSize(container, plotlyLayout, options)).process(container);
})
);
})
)
.catch(handleError);

const result = {
initialized: promise.then(() => result),
setZoomEnabled: createSafeFunction(allowZoom => {
const layoutUpdates = { dragmode: allowZoom ? "zoom" : false };
return Plotly.relayout(container, layoutUpdates);
}),
destroy: createSafeFunction(() => {
isDestroyed = true;
container.removeAllListeners("plotly_restyle");
unwatchResize();
delete container.__previousSize; // added by `updateChartSize`
Plotly.purge(container);
}),
};

return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
"traceorder": "normal"
}
},
"series": [{ "name": "a" }, { "name": "b", "yaxis": "y2" }]
"series": [
{ "name": "a" },
{ "name": "b", "yaxis": "y2" }
]
},
"output": {
"layout": {
Expand All @@ -33,15 +36,15 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
},
"yaxis2": {
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0],
"autorange": true,
"range": null,
"overlaying": "y",
"side": "right"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
"traceorder": "normal"
}
},
"series": [{ "name": "a" }, { "name": "b", "yaxis": "y2" }]
"series": [
{ "name": "a" },
{ "name": "b", "yaxis": "y2" }
]
},
"output": {
"layout": {
Expand All @@ -31,15 +34,15 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
},
"yaxis2": {
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0],
"autorange": true,
"range": null,
"overlaying": "y",
"side": "right"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"automargin": true,
"title": null,
"type": "linear",
"autorange": false,
"range": [0, 0]
"autorange": true,
"range": null
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions viz-lib/src/visualizations/chart/plotly/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import heatmap from "plotly.js/lib/heatmap";
import prepareData from "./prepareData";
import prepareLayout from "./prepareLayout";
import updateData from "./updateData";
import updateLayout from "./updateLayout";
import applyLayoutFixes from "./applyLayoutFixes";
import updateYRanges from "./updateYRanges";
import updateChartSize from "./updateChartSize";
import { prepareCustomChartData, createCustomChartRenderer } from "./customChartUtils";

Plotly.register([bar, pie, histogram, box, heatmap]);
Expand All @@ -22,8 +22,8 @@ export {
prepareData,
prepareLayout,
updateData,
updateLayout,
applyLayoutFixes,
updateYRanges,
updateChartSize,
prepareCustomChartData,
createCustomChartRenderer,
};
Loading

0 comments on commit a1255b4

Please sign in to comment.