-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
[explorev2] Make chart container more responsive #1724
[explorev2] Make chart container more responsive #1724
Conversation
**Leave chart empty when theres error **Make all boolean fields auto-query chart when changed
style={{ 'overflow-x': 'scroll' }} | ||
/>) | ||
} | ||
{this.renderChart(this.props.alert, this.props.isChartLoading)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I'm missing something but you shouldn't have to pass props as they'll be in scope in the function
}); | ||
const refreshChart = Object.keys(nextProps.form_data).some((field) => ( | ||
nextProps.form_data[field] !== this.props.form_data[field] | ||
&& (typeof nextProps.form_data[field] === 'boolean' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean you assume that all boolean fields should act like an autoQueryFields
? It may not be the case. I think you probably want to add these fields in the autoQueryFields
array instead.
Nice, LGTM. eventually we should have a way to tell the user that a field is an |
Maybe at some point we could reorganize Control Panel for each viz_type so that 'Chart Options' are grouped together as one section and every field in it is auto-query |
Before:
the old chart stays in chart container when there's error drawing new chart
Done:
**Leave chart empty when theres error
**Make all boolean fields auto-query chart when changed
needs-review @ascott