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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove minimum height for zero-values in bar chart #209

Merged
merged 1 commit into from
Jul 21, 2019
Merged

Remove minimum height for zero-values in bar chart #209

merged 1 commit into from
Jul 21, 2019

Conversation

Alchez
Copy link
Contributor

@Alchez Alchez commented Dec 19, 2018 •
edited

Explanation About What Code Achieves:

Fixes #201.

This is really a UX issue.

Previously, if any datapoint was a zero-value, it would still be visible on the chart, making it seem it has non-zero data.

Screenshots/GIFs:

Please pardon the insane precision on the values 馃槃

Before:

image

After:

image

Steps To Test:
  • Refer to a bar or stacked bar chart with zero values - they shouldn't appear on the chart.
TODOs:
  • I wasn't able to run the contribution steps locally, and I got the following error on npm run dev:

image

So I have no idea how to rebuild the distribution(s). That might need a separate commit/PR.

@Alchez
Remove minimum height for zero-values in bar chart
87b68e9 
Fixes frappe#201.

This is really a UX issue.

Previously, if any datapoint was a zero-value, it would still be visible on the chart, making it seem it has non-zero data.

P.S. I wasn't able to run the contribution steps locally, so I have no idea how to rebuild the distribution(s). That might need a separate commit/PR.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 76

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.158%
Totals Coverage Status
Change from base Build 71: 0.0%
Covered Lines: 29
Relevant Lines: 62
馃挍 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 76

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.158%
Totals Coverage Status
Change from base Build 71: 0.0%
Covered Lines: 29
Relevant Lines: 62
馃挍 - Coveralls

@scmmishra scmmishra self-assigned this Jun 11, 2019
@scmmishra scmmishra merged commit 3cedc2a into frappe:master Jul 21, 2019
@scmmishra scmmishra mentioned this pull request Jul 21, 2019
Closed
@Alchez Alchez deleted the patch-1 branch July 21, 2019 16:12
This was referenced Oct 8, 2019
Closed
Closed
@snyk-bot snyk-bot mentioned this pull request Oct 24, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@scmmishra scmmishra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

0 valuse in stacked bar
3 participants
@Alchez @coveralls @scmmishra