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

Support hovertemplate on hoveron: fills #6121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fredricj
Copy link

Scatter trace currently doesn't hovertemplate on hoveron: fills. Change this to allow hovertemplate on fills when mode=='lines'

Scatter trace currently doesn't hovertemplate on hoveron: fills. Change this to allow hovertemplate on fills when mode=='lines'
@archmoj archmoj added status: reviewable community community contribution feature something new labels Mar 10, 2022
@archmoj
Copy link
Contributor

archmoj commented Mar 23, 2022

Looking good to me.
@fredricj could you please add a test here to lock the behavior?
Also please create draftlogs/6121_add.md file as described here.
Thank you!

@@ -76,7 +76,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
dfltHoverOn.push('fills');
}
coerce('hoveron', dfltHoverOn.join('+') || 'points');
if(traceOut.hoveron !== 'fills') coerce('hovertemplate');
coerce('hovertemplate', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

We could oossibly use

Suggested change
coerce('hovertemplate', false);
coerce('hovertemplate');

so that hovertemplate defaults to blank string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we keep the initial if statement and just add a condition for lines mode?

if(traceOut.hoveron !== 'fills' || traceOut.mode === 'lines') coerce('hovertemplate');

@archmoj
Copy link
Contributor

archmoj commented Jul 13, 2024

@fredricj Are you interested in completing this PR?

@archmoj archmoj modified the milestone: v2.34.0 Jul 13, 2024
@fredricj
Copy link
Author

We are using this feature on our site so getting it in would be great. I did for some reason miss the additional comments you made, but there is also some other issue I discovered when creating tests which I didn't have time to resolve. Right now I don't remember what so I will need to revisit the changes again and see what it was.

@archmoj archmoj added this to the v2.35.0 milestone Jul 19, 2024
@archmoj
Copy link
Contributor

archmoj commented Jul 19, 2024

@fredricj FYI - I added this PR to our v2.35.0 milestone. Let's try to make it to the finish line.
Please fetch upstream/master and merge it into this branch to have the latest tests updates.
Thank you!

@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson added P2 needed for current cycle and removed status: has TODOs labels Aug 8, 2024
@archmoj archmoj modified the milestones: v2.35.0, v2.36.0 Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new P2 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants