Add option for BigNumber to not start y-axis at 0

[kristw]

With the recent change to BigNumber, the BigNumber with trend line will start y-axis at 0 by default. However, this behavior may be not desirable in some situation, so this PR add an option to overwrite that and start y-axis at min value instead.

@graceguo-supercat @williaster

[codecov-io]

Codecov Report

Merging #5552 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #5552      +/-   ##
==========================================
- Coverage   63.36%   63.36%   -0.01%     
==========================================
  Files         351      351              
  Lines       22258    22259       +1     
  Branches     2470     2470              
==========================================
  Hits        14104    14104              
- Misses       8139     8140       +1     
  Partials       15       15

Impacted Files	Coverage Δ
superset/assets/src/explore/visTypes.jsx	57.14% <ø> (ø)	⬆️
superset/assets/src/explore/controls.jsx	46.96% <ø> (ø)	⬆️
superset/assets/src/visualizations/BigNumber.jsx	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7f9dab...0ec8664. Read the comment docs.

[john-bodley]

Thanks for adding this. My only question is about the language as I think we may have a similar feature elsewhere and thus it would be great if this was consisent.

[williaster]

I believe we have a y-min/max setting for the nvd3 line chart, but I think a boolean is better here.

In person we discussed actually preferring the option "start y-axis at 0" (rather than the negated version it is now), the issue is that we want the default to be true in that case, and since slices don't have this parameter yet it would default to falsy.

In thinking about this more though, we could use the preferred "start y-axis at 0," and then check if the value === false. that way, although current slices would have an undefined, it would result in a 0 y-axis as we want because === false is false. thoughts?

[mistercrunch]

@williaster controls have a default attribute that should get populated properly for all charts saved pre-existing the control. This should work well for either true/false and both in the context of the dashboard and the explore view.

But yes, the default should be to not be 0-bound since this is the previous behavior and much like a sparkline: we want to highlight how that big number is trending.

[kristw]

@mistercrunch I tried setting the default to true and explore the existing chart. However, the checkbox remains unchecked by default. The value received is false. Doing some investigation on that.

[mistercrunch]

@kristw make sure to follow go vanilla from following a plain link from a chart that was save before the control existed. If you had loaded the chart prior to changing/setting the default, the value may have gotten materialized in the URL, and then stays when refreshing the page.

[kristw]

I clicked on "Explore chart" from a dashboard

[mistercrunch]

@kristw from my understanding, this function https://github.com/apache/incubator-superset/blob/master/superset/assets/src/explore/store.js#L102 should be applying defaults to values that are missing. You may have to debug why that's not the case.

[hughhhh]

Can we use positive logic here. Instead of do_not_start_y_axis_at_zero can we make it start_y_axis_at_zero. Changing the name you can pull off the ! here

I feel like most users actually prefer the Y axis to be set to the min value so we can leave default as false.

[kristw]

Somehow after restarting machine the default value works. Must be glitch on my machine. Thanks @mistercrunch

[williaster]

LGTM