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

Multiple responsive rules apply almost randomly when in ascending order #6291

Closed
stpoa opened this Issue Jan 24, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@stpoa

stpoa commented Jan 24, 2017

Expected behaviour

After resizing charts container responsive rules should apply without problems.

Actual behaviour

After resizing charts container there are many problems with responsive rules. Sometime we see that no rule applies at all, sometimes we see that wrong rule is applied.

Live demo with steps to reproduce

https://jsfiddle.net/g0ryva91/

  1. Look at responsive rules
  2. Click 100px, 200px, 300px, 400px
  3. Notice Chart.title change

@TorsteinHonsi TorsteinHonsi added the Bug label Jan 27, 2017

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 27, 2017

Collaborator

It seems like rule matches from the last run survives and interferes with the rule order. If we click one button at the time, with reloading in between each, it is correct.

In short, all buttons except 500px should apply the maxHeight: 400 rule, because that's the last rule that matches the condition.

Collaborator

TorsteinHonsi commented Jan 27, 2017

It seems like rule matches from the last run survives and interferes with the rule order. If we click one button at the time, with reloading in between each, it is correct.

In short, all buttons except 500px should apply the maxHeight: 400 rule, because that's the last rule that matches the condition.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 27, 2017

Collaborator

The problem lies in the matchResponsiveRule function. The rules are evaluated from top to bottom. Each rule either matches or not. If the rule matches, the chart is updated with its options, and the existing options are stored. If the rule fails but was applied earlier, the chart is updated with the stored options from the previous step.

And that's where it fails. Click 100px, 300px and 500px in succession. When we click first 100px, the last rule that applies is the 400px rule, but at this point the title is 300px that was set by the previous rule, so this is the restore-option that is saved for 400px. So when we click 500px, the 400px rule no longer applies, and it sets the title back to 300px.

Collaborator

TorsteinHonsi commented Jan 27, 2017

The problem lies in the matchResponsiveRule function. The rules are evaluated from top to bottom. Each rule either matches or not. If the rule matches, the chart is updated with its options, and the existing options are stored. If the rule fails but was applied earlier, the chart is updated with the stored options from the previous step.

And that's where it fails. Click 100px, 300px and 500px in succession. When we click first 100px, the last rule that applies is the 400px rule, but at this point the title is 300px that was set by the previous rule, so this is the restore-option that is saved for 400px. So when we click 500px, the 400px rule no longer applies, and it sets the title back to 300px.

@mynameiswhat1

This comment has been minimized.

Show comment
Hide comment
@mynameiswhat1

mynameiswhat1 Jan 27, 2017

Hello,
I've noticed some more erratic behaviour that maybe is related with this bug as described here:

http://forum.highcharts.com/highstock-usage/responsive-rules-working-very-strangelly-t37604/

Do you have an idea when this will be fixed?
Thank you very much.

mynameiswhat1 commented Jan 27, 2017

Hello,
I've noticed some more erratic behaviour that maybe is related with this bug as described here:

http://forum.highcharts.com/highstock-usage/responsive-rules-working-very-strangelly-t37604/

Do you have an idea when this will be fixed?
Thank you very much.

@mynameiswhat1

This comment has been minimized.

Show comment
Hide comment
@mynameiswhat1

mynameiswhat1 Jan 27, 2017

Hello again,
Is this also related: http://forum.highcharts.com/post131726.html#p131726 ?
Thank you very much.

mynameiswhat1 commented Jan 27, 2017

Hello again,
Is this also related: http://forum.highcharts.com/post131726.html#p131726 ?
Thank you very much.

@mynameiswhat1

This comment has been minimized.

Show comment
Hide comment
@mynameiswhat1

mynameiswhat1 Jan 30, 2017

Hello,
Any news on this subject?
Thank you very much.

mynameiswhat1 commented Jan 30, 2017

Hello,
Any news on this subject?
Thank you very much.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 30, 2017

Collaborator

Will continue to work on it now.

Collaborator

TorsteinHonsi commented Jan 30, 2017

Will continue to work on it now.

@mynameiswhat1

This comment has been minimized.

Show comment
Hide comment
@mynameiswhat1

mynameiswhat1 Jan 30, 2017

Thank you TorsteinHonsi.
Will this change also take effect on Highstock?
Thank you very much.

mynameiswhat1 commented Jan 30, 2017

Thank you TorsteinHonsi.
Will this change also take effect on Highstock?
Thank you very much.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Jan 31, 2017

Collaborator

You're welcome. Yes, Highstock uses the same code.

Collaborator

TorsteinHonsi commented Jan 31, 2017

You're welcome. Yes, Highstock uses the same code.

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