Skip to content

fix(bar): prevent outside text labels from overlapping tilted axis ticks - #7872

Open
vizansh wants to merge 9 commits into
plotly:masterfrom
vizansh:fix-bar-label-overlap
Open

fix(bar): prevent outside text labels from overlapping tilted axis ticks#7872
vizansh wants to merge 9 commits into
plotly:masterfrom
vizansh:fix-bar-label-overlap

Conversation

@vizansh

@vizansh vizansh commented Jun 27, 2026

Copy link
Copy Markdown

Fixes #7822

This pull request dynamically calculates the positioning for bar chart text labels when textposition='outside'.

Instead of using a static pixel margin, it looks up the active axis configuration and utilizes .getBBox() to measure the exact live dimensions of the tilted or long axis labels. It then shifts the bar numbers by that precise footprint plus a clean visual buffer, entirely preventing overlaps with the top/right axis ticks.

@camdecoster

Copy link
Copy Markdown
Contributor

Thanks for the PR! Our team will review and provide feedback.

@vizansh

vizansh commented Jul 14, 2026

Copy link
Copy Markdown
Author

Hi maintainers, could someone please approve and trigger the GitHub Actions workflow for this PR? I'd love to make sure all the automated CI/CD tests pass successfully. Thank you!

@camdecoster

Copy link
Copy Markdown
Contributor

Until we get to this review, could you please look into the test failures and see if anything is broken/needs to be updated?

@emilykl

emilykl commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Hi @vizansh ! I just spent some time reviewing the original issue #7822, and posted a comment with my thoughts on the root cause and the best approach for a fix. I think the best fix will require a different approach than what you've done here.

Please feel free to continue working and pursue one of the fix approaches outlined in my comment if you like, or propose another approach. I'm happy to review.

@emilykl emilykl assigned vizansh and unassigned emilykl Jul 15, 2026
@vizansh

vizansh commented Jul 29, 2026

Copy link
Copy Markdown
Author

/review

Comment thread src/traces/bar/plot.js Outdated
Comment thread src/traces/bar/plot.js
Comment thread src/traces/bar/plot.js Outdated
Comment thread src/traces/bar/plot.js Outdated
Comment thread test/jasmine/tests/bar_test.js
Comment thread src/traces/bar/plot.js Outdated

@emilykl emilykl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vizansh Nice work! I think this is ready to merge once my comments are addressed.

Comment thread draftlogs/7872_fix.md Outdated
vizansh and others added 3 commits July 30, 2026 18:36
Co-authored-by: Emily KL <4672118+emilykl@users.noreply.github.com>
Co-authored-by: Emily KL <4672118+emilykl@users.noreply.github.com>
@vizansh
vizansh requested a review from emilykl July 30, 2026 14:29
@vizansh

vizansh commented Jul 30, 2026

Copy link
Copy Markdown
Author

Hi @emilykl! Thank you so much for the detailed review and guidance! Those remaining inline errors were just leftover artifacts from my very first commit attempt. All of your suggested refactors and cleanups have been applied in the latest commits. Ready for another review whenever you have a moment! Thanks again for all your help!

Comment thread src/traces/bar/plot.js
Comment on lines +968 to +971
targetX = x1 - dir * (textpad + axisPad);
anchorX = dir * extrapad;
} else {
targetY = y1 + dir * textpad;
targetY = y1 + dir * (textpad + axisPad);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
targetX = x1 - dir * (textpad + axisPad);
anchorX = dir * extrapad;
} else {
targetY = y1 + dir * textpad;
targetY = y1 + dir * (textpad + axisPad);
targetX = x1 - dir * textpad;
anchorX = dir * extrapad;
} else {
targetY = y1 + dir * textpad;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vizansh I've added a new comment with the suggestion, could you apply it? The reference to axisPad should be removed.

Comment thread src/traces/bar/plot.js Outdated
isHorizontal: isHorizontal,
constrained: constrained,
angle: angle
angle: angle,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
angle: angle,
angle: angle,
zeroBarDir: zeroBarDir

angle should be added back in here. My fault, I mistakenly suggested deleting it.

Comment thread src/traces/bar/plot.js
isHorizontal: isHorizontal,
constrained: constrained,
angle: angle
zeroBarDir: zeroBarDir

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
zeroBarDir: zeroBarDir
angle: angle,
zeroBarDir: zeroBarDir

angle should be added back in here. My fault, I mistakenly suggested deleting it.

@emilykl

emilykl commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

@vizansh A couple more cleanup suggestions to apply, then this should be good. I tried applying them myself but got an error so hopefully it works on your side. Thanks for the quick response!

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.

[BUG]: Bar labels overlap with top/right axis labels with textposition=outside

3 participants