-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
David/enhancement modebar #7429
New issue
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
base: master
Are you sure you want to change the base?
Conversation
So I'm wondering what are the configuration options for the modebar besides hover, and what actually changes the visibility during hover (is it just css? can we do the same thing for focus as hover). Also looks like your editor did a bunch of reformatting, you should undo that Also, if we use button, we don't need tabindex now, right? |
Yeah, so, we have two options:
I prefer 2 but we should do one or the other |
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 are the configuration options for the modebar? can we fix the formatting? can we choose one of two accessibility strategies?
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.
also we need to fix the dist/ build/ folders, ill figure out who I should cc
@gvwilson this is an accessibility which makes the modebar tab-able (some of the buttons require a mouse but not all of them). Can someone on your team clarify for us what we need to run to fix the build/ and dist/ folders to submit the PR properly? @davidangarita1, I'd like some help understand the modebar a little better. Do you understand how its configured and what its options are? |
I'm interested in changing the modebar through configuration |
@ayjayt here is the config an options: |
Yeah so let's just make sure no options interfere with the accessibility setup and vice versa |
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.
just some questions
@@ -25,6 +29,11 @@ | |||
position: relative; | |||
vertical-align: middle; | |||
white-space: nowrap; | |||
|
|||
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.
@davidangarita1 do we still use this?
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.
By converting the Anchor to Button they were misaligned, this style is to focus the Plotly logo into the mode bar
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.
i mean the anchor tag, is there still an anchor tag?
edit: ok, i get it
|
||
svg { | ||
position: relative; | ||
top: 2px; |
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.
@davidangarita1 why did we get rid of this?
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.
By converting the anchor to Button they are misaligned, this style is that the buttons icon is centered
Thank you for this work @davidangarita1 ! This is huge! In Chrome I was able to tab through the modebar buttons and use Enter/Space to activate the ones that don't require mouse input 🎉 Is there additional functionality I should try out? Couple things I noticed, you can let me know if you see this behavior as well:
BTW, in case it's helpful for testing, if you add this key/value pair to the plot's modeBarButtonsToAdd: ["sendDataToCloud", "editInChartStudio", "zoom2d", "pan2d", "select2d", "lasso2d", "drawclosedpath", "drawopenpath", "drawline", "drawrect", "drawcircle", "eraseshape", "zoomIn2d", "zoomOut2d", "autoScale2d", "resetScale2d", "hoverClosestCartesian", "hoverCompareCartesian", "zoom3d", "pan3d", "orbitRotation", "tableRotation", "resetCameraDefault3d", "resetCameraLastSave3d", "hoverClosest3d", "zoomInGeo", "zoomOutGeo", "resetGeo", "hoverClosestGeo", "hoverClosestGl2d", "hoverClosestPie", "resetViewSankey", "toggleHover", "resetViews", "toggleSpikelines", "resetViewMapbox", "resetViewMap", "zoomInMapbox", "zoomInMap", "zoomOutMapbox", "zoomOutMap"] |
@emilykl Thank you so much for the thoughtful feedback — I really appreciate it! Regarding the "Autoscale" and "Home" buttons: those should reset the chart when zooming or panning has occurred. So if you zoom into the plot, pressing those buttons (via Enter/Space or mouse) should restore the axes to their default ranges. Let me know if that's not happening for you! Great point as well about testing with all the modebar buttons. When enabling all of them, keep in mind that some — like "Draw closed freeform" — are not keyboard-accessible even if they receive focus. That’s because their functionality depends on direct mouse interaction to draw on the chart, so triggering them via keyboard won’t result in any visible action. As for Safari, I’ll look into compatibility with the version you mentioned (Safari 17) to see if recent changes are working properly there. Thanks again for testing this so thoroughly — it's super helpful! |
Pull request to improve accessibility for the modebar.
This makes the modebar tab-able so that people using special interfaces can interact with the graph. Some functions require a mouse, but not all do.