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

Remove YAxis Custom Min/Max Check for Padding #3982

Merged
merged 1 commit into from
May 2, 2018

Conversation

almic
Copy link
Contributor

@almic almic commented Apr 27, 2018

I noticed that while trying to add spacing to the bottom of a chart, to allow for the interior labels enough room that they don't overlap the data, nothing was happening. I was confused as to why setting the bottom spacing was doing literally nothing, and discovered it was disabled if the min was manually changed.

I thought it strange to disable this without mentioning anywhere in the documentation that this behavior was intentional. Adding percentage based padding let's developers easily add this extra space, but disabling it doesn't make sense. If developers don't want that extra spacing, they'll do what they would've done anyway, set the spacing to 0, as the default is 10%.

Since this behavior of disabling the spacing if custom values are set, even though the calculated range is BASED on those custom values, is not mentioned in documentation, I doubt this will cause any current compatibility issues, especially since the change will likely not be visually noticeable in the first place.

I really hope you can push this into the main library, as I think it makes more logical sense this way. Currently, I'm just re-calculating the min value myself, and then setting the new min with padding.

calculate() no longer checks if min and max is custom, it just adds the padding.
@almic almic changed the title Remove YAxis Custom Check Remove YAxis Custom Min/Max Check for Padding Apr 27, 2018
@almic
Copy link
Contributor Author

almic commented May 2, 2018

Well since no one has commented on this I guess it's cool to add but whatever 😞

@almic almic merged commit 89e50c8 into PhilJay:master May 2, 2018
regas99 pushed a commit to regas99/MPAndroidChart that referenced this pull request Apr 1, 2019
Remove YAxis Custom Min/Max Check for Padding
@danielgindi
Copy link
Collaborator

This should be reverted. What it does is strip customAxisMin/Max of its meaning.
Because you are recalculating it based on the auto-calculated min/max + spacing, where custom means a completely different value set manually.

You could take this into account in a different way, but your implementation actually hurts a fundamental feature badly.

@danielgindi
Copy link
Collaborator

I see that it was reverted in a later PR.

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.

2 participants