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

Fix legend item layout issue #5816

Merged
merged 1 commit into from
Nov 12, 2018
Merged

Fix legend item layout issue #5816

merged 1 commit into from
Nov 12, 2018

Conversation

nagix
Copy link
Contributor

@nagix nagix commented Nov 6, 2018

When layout.padding is specified, legend item layout goes wrong, and some items disappear or overlap with the chart.

In the the following examples, I added a blue frame to the legend box and red frames to the legend item boxes to make it easier to understand each box's position.

Version 2.7.3: https://jsfiddle.net/nagix/dy1etr8h/

The legend item for the dataset 'G' disappears in the first chart, and it overlaps with the chart in the second chart.

screenshot 2018-11-07 at 3 25 10 am

screenshot 2018-11-07 at 3 25 27 am

This PR: https://jsfiddle.net/nagix/4b8utyLv/

In this PR, calculation for item layout is fixed, and legend.labels.padding is correctly applied to the left most items.

screenshot 2018-11-07 at 3 25 54 am

screenshot 2018-11-07 at 3 26 11 am

Fixes #4112
Fixes #5491
Fixes #5606

@nagix nagix force-pushed the issue-5816 branch 4 times, most recently from eb26a7e to a1c8533 Compare November 6, 2018 21:40
@benmccann
Copy link
Contributor

Does this fix also work in the case that there are three rows of labels?

@nagix
Copy link
Contributor Author

nagix commented Nov 9, 2018

Yes, you can see animations in the fiddle and how it works when the legend box becomes narrow.

@simonbrunel simonbrunel added this to the Version 2.8 milestone Nov 12, 2018
@simonbrunel simonbrunel merged commit e730f87 into chartjs:master Nov 12, 2018
@simonbrunel
Copy link
Member

Thanks @nagix

@nagix nagix deleted the issue-5816 branch November 12, 2018 11:15
@simonbrunel
Copy link
Member

@nagix some of these tests now fail on my machine, what is your dev environment?

@nagix
Copy link
Contributor Author

nagix commented Nov 15, 2018

My environment is Mac OS. The cause of the failures is a floating point number error, correct?

@simonbrunel
Copy link
Member

I'm not sure:

Expected 105 to be close to pixel 107.
@test/specs/plugin.legend.tests.js:213:48

Expected 53 to be close to pixel 56.
@test/specs/plugin.legend.tests.js:466:48

Expected 232 to be close to pixel 228.7.
@test/specs/plugin.legend.tests.js:493:30

Expected 121 to be close to pixel 119.
@test/specs/plugin.legend.tests.js:522:48

Expected 101 to be close to pixel 99.
@test/specs/plugin.legend.tests.js:524:49

@simonbrunel
Copy link
Member

Firefox only (63.0.1), it works on Chrome

@nagix
Copy link
Contributor Author

nagix commented Nov 15, 2018

Ah, I'm sure this is due to the difference in the font size.

@simonbrunel
Copy link
Member

Any idea how to make these tests more stable? maybe by setting empty labels for each dataset or find a stable character to repeat, or ...

@nagix
Copy link
Contributor Author

nagix commented Nov 16, 2018

The use of empty labels would be simplest. I will open a new PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants