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 line chart overflowing the right side #6829

Merged
merged 3 commits into from
Feb 7, 2019
Merged

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Feb 7, 2019

Incorrect margin calculation for line chart making the content overflow right side.
Fix bug introduced by #6626

@betodealmeida @williaster @graceguo-supercat

@kristw kristw added !deprecated-label:bug Deprecated label - Use #bug instead .vis v0.30 labels Feb 7, 2019
Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to commit the package-lock.json?

@kristw
Copy link
Contributor Author

kristw commented Feb 7, 2019

Maybe I should update the package-lock.json in a separate PR to make this one easier to cherry-pick.

@kristw kristw merged commit 823555e into apache:master Feb 7, 2019
@kristw kristw deleted the kristw--line branch February 7, 2019 05:28
if (staggerLabels) {
margins.bottom = 40;
} else {
if (xLabelRotation === 45) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kristw what was the problem with the code? I thought that since xLabelRotation and staggerLabels are mutually exclusive we could check for stagger labels first. And if no rotation is set (ie, xLabelRotation = 0) then lines 647-651 would have no effect since we now compute the margins based on the angle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short ans: I changed and it fixes the issue with line chart.
Longer ans: There are other code that modify margins.right above, which will be overridden by your else clause.

graceguo-supercat pushed a commit that referenced this pull request Feb 8, 2019
* Fix line chart overflowing the right side

* revert package-lock.json

* revert again

(cherry picked from commit 823555e)
@xtinec xtinec added the v0.31 label Feb 8, 2019
xtinec pushed a commit that referenced this pull request Feb 8, 2019
* Fix line chart overflowing the right side

* revert package-lock.json

* revert again

(cherry picked from commit 823555e)
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 2024
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* Fix line chart overflowing the right side

* revert package-lock.json

* revert again

(cherry picked from commit 3f3b7bb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels !deprecated-label:bug Deprecated label - Use #bug instead v0.30 v0.31 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants