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.

Sign up for GitHub

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

Custom JS code chart improvements #1873

Merged
merged 5 commits into from
Sep 13, 2017
Merged

Custom JS code chart improvements #1873

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3fd90c6
Fix: Custom code didn't load into editor
deecay Jul 11, 2017
73466dc
Make custom js textarea resizable
deecay Jul 11, 2017
605a70d
Add toggle for automatically updating graph
deecay Jul 11, 2017
aacc4b7
Fix: Custom code keeps appending trace per refresh
deecay Jul 11, 2017
1a8078a
Fix: Custom code keeps appending trace per refresh
deecay Jul 31, 2017
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
3 changes: 3 additions & 0 deletions client/app/assets/css/superflat_redash.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1781,6 +1781,9 @@ fieldset[disabled] .form-control {
textarea.form-control {
height: auto;
}
textarea.v-resizable {
resize: vertical;
}
input[type="search"] {
-webkit-appearance: none;
}
Expand Down
18 changes: 12 additions & 6 deletions client/app/visualizations/chart/chart-editor.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,23 @@

<div class="form-group" ng-if="options.globalSeriesType == 'custom'">
<label class="control-label">Custom code</label>
<textarea ng-model="options.customCode" class="form-control" rows="10">
<textarea ng-model="options.customCode" class="form-control v-resizable" rows="10">
</textarea>
</div>

<div class="checkbox" ng-if="options.globalSeriesType == 'custom'">
<label>
<input type="checkbox" ng-model="options.enableConsoleLogs">
<i class="input-helper"></i> Show errors in the console
</label>
</div>
<label>
<input type="checkbox" ng-model="options.enableConsoleLogs">
<i class="input-helper"></i> Show errors in the console
</label>
</div>

<div class="checkbox" ng-if="options.globalSeriesType == 'custom'">
<label>
<input type="checkbox" ng-model="options.autoRedraw">
<i class="input-helper"></i> Auto update graph
</label>
</div>

<div ng-show="currentTab == 'xAxis'">
<div class="form-group">
Expand Down
4 changes: 3 additions & 1 deletion client/app/visualizations/chart/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,12 @@ function ChartEditor(ColorPalette, clientConfig) {

scope.showSizeColumnPicker = () => some(scope.options.seriesOptions, options => options.type === 'bubble');

scope.options.customCode = `// Available variables are x, ys, element, and Plotly
if (scope.options.customCode === undefined) {
scope.options.customCode = `// Available variables are x, ys, element, and Plotly
// Type console.log(x, ys); for more info about x and ys
// To plot your graph call Plotly.plot(element, ...)
// Plotly examples and docs: https://plot.ly/javascript/`;
}

function refreshColumns() {
scope.columns = scope.queryResult.getColumns();
Expand Down
9 changes: 7 additions & 2 deletions client/app/visualizations/chart/plotly.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,9 @@ const CustomPlotlyChart = (clientConfig) => {
return;
}
const refresh = () => {
// Clear existing data
element[0].children[0].calcdata = [];
element[0].children[0].data = [];
Copy link
Member

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This piece is related to

  1. Fix: Any update of the graph in editor causes appended trace.

and used by

        const codeCall = eval(`codeCall = function(x, ys, element, Plotly){ ${scope.options.customCode} }`);
        codeCall(scope.x, scope.ys, element[0].children[0], Plotly);

The code above appends a trace to an already existing plot. So without clearing data from the already drawn plotly graph, every update in code editor will cause the trace in the graph to multiply. One-trace bar graph becomes two-trace bar graph, three-trace bar graph, and so on, each time you type in code editor.

Using Plotly.newPlot() might be another option to avoid appending, but haven't tried it yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see 1a8078a .

This less hackish way fixes the problem too.

// eslint-disable-next-line no-eval
const codeCall = eval(`codeCall = function(x, ys, element, Plotly){ ${scope.options.customCode} }`);
codeCall(scope.x, scope.ys, element[0].children[0], Plotly);
Expand All @@ -514,9 +517,11 @@ const CustomPlotlyChart = (clientConfig) => {
});
});
};
scope.$watch('options.customCode', () => {
scope.$watch('[options.customCode, options.autoRedraw]', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to watch autoRedraw? If it's disabled, on next refresh it won't redraw. If it's enabled, still we have nothing to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so too. But I realized that I would want the graph to redraw 'when I ticked the auto-redraw'. Without this watch, only the next change in code will update the graph.

I think, writing a lengthy code without autoRadraw, then placing a tick on autoRedraw, should trigger the graph redraw immediately.

Copy link
Member

Choose a reason for hiding this comment

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

Then why not just watch option.autoRedraw?

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 I am editing a small script, I would like to refresh graph as I type. This is done by watching customCode.
When I am editing a big script, I would like to turn off the refresh-as-I-type, but when I click on the auto redraw checkbox, I want so see the graph refreshed immediately. This last part is done by watching autoRedraw.

try {
refresh();
if (scope.options.autoRedraw) {
refresh();
}
} catch (err) {
if (scope.options.enableConsoleLogs) {
// eslint-disable-next-line no-console
Expand Down