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

Bars Overlap axis When minValue is greater than 0. #1188

Closed
ollwenjones opened this issue Oct 27, 2020 · 8 comments · Fixed by #1342
Closed

Bars Overlap axis When minValue is greater than 0. #1188

ollwenjones opened this issue Oct 27, 2020 · 8 comments · Fixed by #1342

Comments

@ollwenjones
Copy link
Contributor

ollwenjones commented Oct 27, 2020

Describe/explain the bug
If bar axis minimum is set much above 0, the bars extend off the edge of the chart and obscure the axis & its labels

To Reproduce
https://codesandbox.io/s/nivobar-chart-overlapping-axis-mod21?file=/src/index.js

Steps to reproduce the behavior:

  1. Create a bar chart with either orientation.
  2. Set minValue to be > 0. 20% or so of smallest entry works best to reproduce.

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
image

Desktop (please complete the following information):

  • OS: Windows
  • Browser Chrome
  • Version 86

Additional context
I might be able to work around this in my app by adjusting the stack order...

@ollwenjones
Copy link
Contributor Author

@plouc mentioned on discord we could clamp this except it's very complex given how flexible the bar behavior is (grouped vs stacked), (bars vs columns,) etc. It seems like it would be straightforward for grouped bars based on orientation. I can see how stacked values would make this more complex, though.

@wyze
Copy link
Contributor

wyze commented Nov 9, 2020

Adding logic in the scales package for clamp on the linearScale seems straight forward. Then you can pass valueProp={{ type: 'linear', clamp: true }} and get desired results. Minor tick overlapping on the right axis, but that's because you specified a 0 tick value so it is getting clamped to the 1 position.

image

@ollwenjones
Copy link
Contributor Author

@wyze super cool. The specified tickValues were just something from the example I grabbed; nothing important. Is that something you did just there in codesandbox, or were there additional changes to the way Bar exposes its scales to do it?

@wyze
Copy link
Contributor

wyze commented Nov 10, 2020

Just a couple changes to scales package. Here

{ axis, min = 0, max = 'auto', stacked = false, reverse = false },
add something like , clamp = false to the first parameter.

Then somewhere down here

add if (clamp) { scale.clamp(true) }.

Update PropTypes in that linearScales file.

Update typings for scales package here

with clamp?: boolean.

That should do it.

@ollwenjones
Copy link
Contributor Author

That is about the friendliest indirect invitation to PR that I have ever received 😄

@wyze
Copy link
Contributor

wyze commented Nov 10, 2020

I wasn't sure if you wanted to contribute or not, don't feel pressured to do so. :)

@ollwenjones
Copy link
Contributor Author

would love to. just racing towards a deadline at present. This is linked in our backlog, so it's not unlikely someone will circle back and contribute :D

@ollwenjones
Copy link
Contributor Author

Hoping to have some time to contribute here, soon.

ollwenjones pushed a commit to decisions-com/nivo that referenced this issue Dec 11, 2020
Optional ability to tell a scale to clamp. use case e.g. setting bar
value below zero

fixes plouc#1188
ollwenjones pushed a commit to decisions-com/nivo that referenced this issue Dec 11, 2020
Optional ability to tell a scale to clamp. use case e.g. setting bar
value below zero

fixes plouc#1188
wyze pushed a commit that referenced this issue Dec 11, 2020
Optional ability to tell a scale to clamp. use case e.g. setting bar
value below zero

fixes #1188

Co-authored-by: Sam Jones <[email protected]>
ddavydov pushed a commit to netronixgroup/nivo that referenced this issue Apr 8, 2021
Optional ability to tell a scale to clamp. use case e.g. setting bar
value below zero

fixes plouc#1188

Co-authored-by: Sam Jones <[email protected]>
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 a pull request may close this issue.

2 participants