Skip to content

Draw BarChart and LineChart from 0#107

Merged
Hermanya merged 2 commits into
chart-kit:masterfrom
k-yokoishi:fix/draw-charts-from-zero
Apr 25, 2019
Merged

Draw BarChart and LineChart from 0#107
Hermanya merged 2 commits into
chart-kit:masterfrom
k-yokoishi:fix/draw-charts-from-zero

Conversation

@k-yokoishi

@k-yokoishi k-yokoishi commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

It's a proposal to make BarChart and LineChart more general UI of the chart.

Currently, the bar chart and the line chart are drawn from the minimum value as follows,

current

however generally these charts are drawn from 0 by default, so I modified it in this PR like below.

modified

@Hermanya

Copy link
Copy Markdown
Contributor

Maybe this could be behind a prop, like rangeFromZero, which is false by default.

The reason why the range currently starts from the minimum value is that in case if the values are too large, then it's difficult to show the difference between values on a range starting from zero. And graphs are all about the difference between values.

@k-yokoishi

Copy link
Copy Markdown
Contributor Author

The reason why the range currently starts from the minimum value is that in case if the values are too large, then it's difficult to show the difference between values on a range starting from zero.

That's right! I agree that it's necessary to show the difference between the values even if those are too large.

But that is usually solved by providing the way to configure the min/max value of y axis label not by drawing charts from the minimum value.

So we can chose some choices:

  1. Draw charts from 0 by default, and provide the way to configure the min/max value of y axis label.
    • I think this choice is ideal because the many chart libraries do so.
  2. Draw charts from the minimum value by default, and add the flag in prop to switch to draw the charts from 0.
    • It doesn't change the current UI by default.

How about your opinion 🤔 In any case, I'll try to fix it!!

@Hermanya

Copy link
Copy Markdown
Contributor

Choice 2 looks good to me. No breaking changes.

@olegberman

Copy link
Copy Markdown
Member

@k-yokoishi Thank you for you input  ❤️

@k-yokoishi

Copy link
Copy Markdown
Contributor Author

Not at all! I'll fix it with the 2nd choice.

Add prop `fromZero` to BarChart and LineChart to
render the charts from 0 not from the minimum value.
@Hermanya

Copy link
Copy Markdown
Contributor

Nice! I'll take a closer look soon 👍

@Hermanya Hermanya self-requested a review April 23, 2019 14:00
@k-yokoishi

Copy link
Copy Markdown
Contributor Author

Please re-review it and try new props, thanks.

@Hermanya Hermanya merged commit f259273 into chart-kit:master Apr 25, 2019
@Hermanya

Copy link
Copy Markdown
Contributor

As usual, great job @k-yokoishi! I appreciate it 😄

@Hermanya

Copy link
Copy Markdown
Contributor

Available in v2.6.0

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.

3 participants