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

softMin causes maxPadding to be ignored for certain cases #18110

Closed
awonihlgard opened this issue Dec 8, 2022 · 3 comments · Fixed by #18981
Closed

softMin causes maxPadding to be ignored for certain cases #18110

awonihlgard opened this issue Dec 8, 2022 · 3 comments · Fixed by #18981
Assignees

Comments

@awonihlgard
Copy link

Expected behaviour

There should be a padding between the serie maximum and the chart maximum that can be controlled with maxPadding

Actual behaviour

maxPadding is not respected, causing the serie to end up to close to the chart edge

Live demo with steps to reproduce

https://jsfiddle.net/zjx86fvm/50/

  • If softMin is replaced with min, the padding is added
  • If the series have a higher variance (e.g replace the first data point with 220), the padding is added

Product version

Highcharts 10.2.0

Affected browser(s)

@highsoft-bot highsoft-bot added this to To do in Development-Flow via automation Dec 8, 2022
@karolkolodziej
Copy link
Contributor

karolkolodziej commented Dec 9, 2022

Hi @awonihlgard, thank you for reporting this issue.

Internal note:
The part responsible for calculating the min/max from softMin/softMax

// Pad the values to get clear of the chart's edges. To avoid
// tickInterval taking the padding into account, we do this after
// computing tick interval (#1337).
if (
!categories &&
!axis.axisPointRange &&
!(axis.stacking && axis.stacking.usePercentage) &&
!isLinked &&
defined(axis.min) &&
defined(axis.max)
) {
length = axis.max - axis.min;
if (length) {
if (!defined(hardMin) && minPadding) {
axis.min -= length * minPadding;
}
if (!defined(hardMax) && maxPadding) {
axis.max += length * maxPadding;
}
}
}
// Handle options for floor, ceiling, softMin and softMax (#6359)
if (!isNumber(axis.userMin)) {
if (
isNumber(options.softMin) && options.softMin < (axis.min as any)
) {
axis.min = hardMin = options.softMin; // #6894
}
if (isNumber(options.floor)) {
axis.min = Math.max(axis.min as any, options.floor);
}
}
if (!isNumber(axis.userMax)) {
if (
isNumber(options.softMax) && options.softMax > (axis.max as any)
) {
axis.max = hardMax = options.softMax; // #6894
}
if (isNumber(options.ceiling)) {
axis.max = Math.min(axis.max as any, options.ceiling);
}
}

is called after the part where the padding is added.

Simply revert that order and run some regression tests.
Needs deeper analysis.

@awonihlgard
Copy link
Author

Any progress on this?

@jakubjanuchta jakubjanuchta self-assigned this May 9, 2023
@jakubjanuchta jakubjanuchta moved this from To do to In progress in Development-Flow May 9, 2023
@highsoft-bot highsoft-bot moved this from In progress to Review in progress in Development-Flow May 9, 2023
@jakubjanuchta jakubjanuchta moved this from Review in progress to In progress in Development-Flow May 9, 2023
@highsoft-bot highsoft-bot moved this from In progress to Review in progress in Development-Flow May 9, 2023
@jakubjanuchta jakubjanuchta moved this from Review in progress to In progress in Development-Flow May 9, 2023
@karolkolodziej
Copy link
Contributor

@awonihlgard We started working on that, please follow the issue to get more updates on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

4 participants