-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Hide hover and spike modebar buttons in plotly.js v2 by default and add config options to bring them back in #5654
Conversation
I would make these all case-insensitive. |
As discussed this morning with @archmoj: Ideally we would collapse all the hovermode buttons to two, each with consistent behavior:
In principle we should be able to do the same with the various type-specific zoom, pan, and reset buttons: define behavior for them that doesn't depend on which subplots are present, so users can just think about which functionality they want present, not which specific functions are attached to those icons. That's probably not in scope right now though, as it'd be a bit of a project. But that seems like what we'd need to do to make these options really user-friendly. |
Hmm seems like we'd really need per-subplot modebars to do this right then. We can discuss for v4 ;) |
Oh I don't think we want per-subplot... except maybe for zoom in/out, but that's really better handled by other more specific interactions anyway. It's just that if you have several subplots with inconsistent modes, neither of the buttons would start out selected, and the modes would be made consistent after you click one of the buttons. But yeah, v3 or v4 🙈 |
Right now having two buttons for |
No need to keep exploring various options in the short term now that these are off by default :) |
if(hoverGroup.indexOf(a) !== -1) { | ||
enabledHoverGroup.push(a); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what you're saying is, we still calculate the hover entries you would have gotten, back in 1.x when we included them by default, and in 2.x you won't get them by default but you'll be allowed to re-include any of those exact buttons but no others.
I guess it makes sense to restrict to only the buttons we know will work, but as a user it may be hard to figure out what those buttons are in any given case. For example Sankey gets ['hoverClosestCartesian', 'hoverCompareCartesian']
? And some multi-type plots just get ['toggleHover']
but others get something more specific?
What if we accept a special buttonsToAdd
value 'hovermode'
, that copies all of hoverGroup
into enabledHoverGroup
- ie to get back 1.x behavior no matter what traces you have you can just use that one special value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we accept a special buttonsToAdd value 'hovermode'
this sounds like a good solution to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe v1hovermode
or something to clearly identify this as "the v1 behaviour" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what you're saying is, we still calculate the hover entries you would have gotten, back in 1.x when we included them by default, and in 2.x you won't get them by default but you'll be allowed to re-include any of those exact buttons but no others.
Exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we accept a special
buttonsToAdd
value'hovermode'
, that copies all ofhoverGroup
intoenabledHoverGroup
- ie to get back 1.x behavior no matter what traces you have you can just use that one special value?
It is doable but at the same time rather confusing (and perhaps hard to maintain) as the v1 hoverGroup
does include spike
options too. Plus in that case we may restrict users to only include all of those buttons but not some?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, if you want some subset of the v1 behavior you have to figure out the specific options you need and specify them explicitly.
Good point re: spikes. 'togglespikelines'
is pretty simple and easy to document; so let's have 'v1hovermode'
give you just the hovermode buttons - ie everything in hoverGroup
except toggleSpikelines
if it's there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
How about allhovermodes
instead of v1hovermode
?
Later on the road we may want to add extra buttons e.g. for unified hover
options and having v1
would make it difficult to add those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we call it allhovermodes
it's wrong until we add unified. And unified is more a decision chart developers will make, whereas compare vs closest has times it's useful to viewers (mainly for charts that the developer didn't explicitly specify the best hovermode) so I don't see much need to add a button for unified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we call it allhovermodes it's wrong until we add unified
Yes. I would favour v1hovermodes
as it's clear that this is simply brining back the old mode without making any additional promises/implying anything else about "all" anything :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in f4e6120.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃 LGTM!
@andoks * plotly.js: add missing hover modebar button strings supported by modeBarButtonsToAdd Add the missing hover-related default-button strings as supported in [plotly.js 2.12][]. Of particular importance is perhaps the v1hovermode which restores the default modebar buttons from plotly v1 (see [PR: Hide hover and spike modebar buttons... #5654][]). Also add a test for using the values in modeBarButtonsToAdd [plotly.js 2.12]: <https://github.com/plotly/plotly.js/blob/v2.12.0/src/components/modebar/manage.js#L235> [PR: Hide hover and spike modebar buttons... #5654]: <plotly/plotly.js#5654> * plotly.js: allow mixing predefined and custom modebar-buttons Also test that passing both predefined button strings and custom button structs works
Addressing #5642.
After this PR, hover and spike line buttons would not be visible by default.
To add any of those back one could provide the name of the button via
config.modeBarButtonsToAdd
similar to what we do for shape drawing buttons.Perhaps we should unify, simplify and nonCamelify some of these names too in v2?
hoverCompareCartesian
hoverClosestCartesian
hoverClosestGl2d
hoverClosest3d
hoverClosestGeo
hoverClosestPie
toggleHover
toggleSpikelines
update:
the pre-defined buttons names are simplified and lowercase as follow:
hovercompare
hoverclosest
togglehover
togglespikelines
In addition one could use
v1hovermode
to bring back v1 hover buttons.@plotly/plotly_js