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

Time-Series Chart Showing Min/Max X-Axis Values Even Though Disabled #15042

Closed
jamesrayoub opened this issue Jun 8, 2021 · 5 comments
Closed
Labels
validation:required A committer should validate the issue viz:charts:timeseries Related to Timeseries

Comments

@jamesrayoub
Copy link

Screenshot

image

Description

By default, the min & max x-axis values appear to be showing despite the fact that they are disabled. I am able to remove by enabling then disabling again using the checkboxes. I'd imagine the default behavior is to not show those values but if it is, then perhaps the checkboxes should be marked by default...

@junlincc junlincc added the bash! label Jun 8, 2021
@junlincc
Copy link
Member

junlincc commented Jun 8, 2021

thanks for reporting!
please see video for more context. @xiezhongfu can you take this on? 🙏
Expected behavior: when user first interact with Timeseries echarts, Min and Max on x-axis should show by default, 2 checkboxes should be checked by default/

Screen.Recording.2021-06-08.at.10.11.08.AM.mov

@junlincc junlincc added the viz:charts:timeseries Related to Timeseries label Jun 8, 2021
@xiezhongfu
Copy link
Contributor

xiezhongfu commented Jun 9, 2021

superset

  • firstly, rendered the table chart which has not xAxisShowMinLabel control.

  • when we chose Time-series Chart, and rendered it. Time-series Chart has a xAxisShowMinLabel control. But the value property is undefined
    image
    image

  • we can set value by mapStateToProps or default function
    image

superset-ui

default is false refer: https://github.com/apache-superset/superset-ui/blob/master/plugins/plugin-chart-echarts/src/Timeseries/controlPanel.tsx#L343
default value of xAxisShowMinLabel is false refer: https://github.com/apache-superset/superset-ui/blob/master/plugins/plugin-chart-echarts/src/Timeseries/types.ts#L93

solution

All in total, we can set value to false by mapStateToProps.

@zhaoyongjie
Copy link
Member

zhaoyongjie commented Jun 10, 2021

I can not reproduce in the latest master and Time-series Chart

image

screen recoding.(since file size over 100M, I uploaded to youtube)
https://www.youtube.com/watch?v=dpLtbZUiDRU

@junlincc
Copy link
Member

@villebro can you clarify what's the expected behavior?

@xiezhongfu
Copy link
Contributor

xiezhongfu commented Jun 10, 2021

document

https://echarts.apache.org/en/option.html#xAxis.axisLabel.showMinLabel.
The options of showMinLabel are true, false, null. I think undefine is auto according to Echarts Source Code.

Echarts Source Code

axisLabel: {
    show: true,
    // Whether axisLabel is inside the grid or outside the grid.
    inside: false,
    rotate: 0,
    // true | false | null/undefined (auto)
    showMinLabel: null,
    // true | false | null/undefined (auto)
    showMaxLabel: null,
    margin: 8,
    // formatter: null,
    fontSize: 12
},

and

// If min or max are user set, we need to check
// If the tick on min(max) are overlap on their neighbour tick
// If they are overlapped, we need to hide the min(max) tick label
const showMinLabel = axisModel.get(['axisLabel', 'showMinLabel']);
const showMaxLabel = axisModel.get(['axisLabel', 'showMaxLabel']);

UI Problem

So, the problem is "UI is not a controlled state. CheckBox will diff showMinLabel"

@junlincc @youngyjd @villebro

xiezhongfu added a commit to xiezhongfu/superset-ui that referenced this issue Jun 20, 2021
xiezhongfu added a commit to xiezhongfu/superset-ui that referenced this issue Jul 3, 2021
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this issue Nov 17, 2021
…el default value (apache#1161)

* fix(plugin-chart-echarts): delete showMinLabel and showMaxLabel

fix apache#15042

* fix(plugin-chart-echarts):
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this issue Nov 24, 2021
…el default value (apache#1161)

* fix(plugin-chart-echarts): delete showMinLabel and showMaxLabel

fix apache#15042

* fix(plugin-chart-echarts):
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this issue Nov 25, 2021
…el default value (apache#1161)

* fix(plugin-chart-echarts): delete showMinLabel and showMaxLabel

fix apache#15042

* fix(plugin-chart-echarts):
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this issue Nov 26, 2021
…el default value (apache#1161)

* fix(plugin-chart-echarts): delete showMinLabel and showMaxLabel

fix apache#15042

* fix(plugin-chart-echarts):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validation:required A committer should validate the issue viz:charts:timeseries Related to Timeseries
Projects
No open projects
Development

No branches or pull requests

4 participants