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

Add support for (hover) 'text' in mesh3d traces #2327

Merged
merged 5 commits into from
Feb 6, 2018
Merged

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Feb 2, 2018

Call this a bug fix or a new feature, this PR closes #2242

@etpinard etpinard added bug something broken feature something new status: reviewable labels Feb 2, 2018
@alexcjohnson
Copy link
Collaborator

Not sure how much you want to let this PR expand, but here's a definitely-a-bug that popped up as I was trying out your PR: hoverinfo has the wrong editType:
Plotly.restyle(gd,{hoverinfo:'x+y'})
does not update the displayed hover info for mesh3d. If you follow it with
Plotly.redraw(gd)
or include something else in the restyle though, it does, like
Plotly.restyle(gd,{hoverinfo:'x+text',text:'AAAAAAAH'})

Also it looks like surface doesn't support text as a single string (only as an array) and both surface and scatter3d have the same hoverinfo restyle problem as mesh3d.

@etpinard
Copy link
Contributor Author

etpinard commented Feb 5, 2018

@alexcjohnson good catch. Thanks.

I think I fixed them all in 8ae2d0e by overriding the hoverinfo attribute declaration with editType: 'calc'.

@alexcjohnson
Copy link
Collaborator

8ae2d0e looks great, thanks for the tests 🔒
Do you want to extend surface to support text as a single string now too, so it matches the new mesh3d.text, or leave that in an issue?

@etpinard
Copy link
Contributor Author

etpinard commented Feb 5, 2018

Do you want to extend surface to support text as a single string now too

Oops. I pushed too soon. That's in d20bdc1

role: 'info',
dflt: '',
arrayOk: true,
editType: 'calc',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh this is in an overrideAll so you don't need editType - which I think applies to a few more of the additions in this PR too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(I see you 🔪 some editType lines, just not the one I made the comment directly on 😅 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Fixed in -> 13a3ecf

@alexcjohnson
Copy link
Collaborator

Nice - slowly getting all these trace types up to par with each other. 💃

@etpinard etpinard merged commit eeef0ed into master Feb 6, 2018
@etpinard etpinard deleted the mesh3d-text branch February 6, 2018 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mesh3d hover text not shown
2 participants