Skip to content

7278 make triangle optional #7451

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

7278 make triangle optional #7451

wants to merge 10 commits into from
+460 −58

Conversation

emilykl
Copy link
Contributor

@emilykl emilykl commented Jul 2, 2025 •
edited
Loading

Closes #7278

Adds trace.hoverlabel.showarrow and layout.hoverlabel.showarrow attributes (default: true) to enable hiding the triangular carat on the hover text box.

Also adds a Jasmine test for the new attributes.

With showarrow: true (default; same as current behavior):
Screenshot 2025-07-22 at 1 32 27 PM

With showarrow: false:
Screenshot 2025-07-22 at 1 32 36 PM

@emilykl emilykl marked this pull request as draft July 2, 2025 16:19
@gvwilson gvwilson added feature something new P1 needed for current cycle labels Jul 3, 2025
@archmoj
Copy link
Contributor

archmoj commented Jul 21, 2025

@emilykl
Is this PR ready for review?
Would you please merge master into this branch?
Thank you 🙏

@emilykl
Copy link
Contributor Author

emilykl commented Jul 22, 2025

@archmoj not quite ready for review but I'll merge master — will ping you later today when ready for review

@emilykl emilykl force-pushed the 7278-make-triangle-optional branch from 1126715 to 63e44ab Compare July 22, 2025 17:27
gvwilson
gvwilson approved these changes Jul 22, 2025
View reviewed changes
src/components/fx/hover.js
'V' + pY(offsetY - HOVERARROWSIZE) +
'Z'));
pathStr = 'M0,0L' + pX(horzSign * HOVERARROWSIZE + offsetX) + ',' + pY(HOVERARROWSIZE + offsetY) +
'v' + pY(d.by / 2 - HOVERARROWSIZE) +
Copy link
Contributor

Choose a reason for hiding this comment

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

OMG math

Copy link
Contributor Author

@emilykl emilykl Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@gvwilson Coming up with the new SVG path string with no arrow was Claude Code's only helpful contribution to this PR (but it's a doozy, so, worth it)

@emilykl emilykl marked this pull request as ready for review July 22, 2025 20:57
@emilykl
Copy link
Contributor Author

emilykl commented Jul 22, 2025 •
edited
Loading

@archmoj This is ready for review. A couple questions for you —

  • I had to remove arrayOk: true for showarrow in 23a8f8a due to a failing test . Looking at plot-schema.json it seems arrayOk is not supported for boolean attributes, does that make sense?

  • It would be nice to support showarrow for parcats traces but it looks like all hoverlabel attributes are not supported for parcats, do you know why? I looked into enabling hoverlabel for parcats but it was not straightforward.

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

@gvwilson gvwilson gvwilson approved these changes

@archmoj archmoj Awaiting requested review from archmoj

@camdecoster camdecoster Awaiting requested review from camdecoster

Assignees

@emilykl emilykl

Labels
feature something new P1 needed for current cycle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

make hover box caret/triangle optional
3 participants
@emilykl @archmoj @gvwilson