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

Custom JS code chart improvements #1873

Merged
merged 5 commits into from
Sep 13, 2017
Merged

Conversation

deecay
Copy link
Contributor

@deecay deecay commented Jul 11, 2017

This PR introduces some fixes and changes to custom code chart visualization.

  1. Fix: Currently, custom js code textarea does not load the saved code properly. This fixes the issue by setting the textarea contents only if the content is undefined.

  2. Fix: Any update of the graph in editor causes appended trace. Trace gets added because the plotly graph is not initialized. Now data and calcdata will be initialized by setting them to [].

  3. Change: Make textarea vertically resizable for readability and editability of code.

  4. Change: Add toggle checkbox for auto-update graph. This is handy when you are dealing with big dataset which takes time to refresh.

This fixes #1628.

deecay added 4 commits July 11, 2017 10:15

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@deecay deecay changed the title Fix: Custom JS code for chart didn't load into editor Custom JS code chart improvements Jul 11, 2017
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Thanks! Please see my comments.

@@ -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.

@@ -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.

@arikfr arikfr merged commit 0a311bf into getredash:master Sep 13, 2017
@arikfr
Copy link
Member

arikfr commented Sep 13, 2017

Thanks!

@deecay deecay deleted the fix-custom-js branch September 13, 2017 13:11
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.

Loading custom JS chart into editor fails.
2 participants