#22237 closed enhancement (fixed)
Add keyboard shortcuts for collapse/expand, panel-back, and close to the Customizer
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | Customize | Keywords: | good-first-bug has-patch commit |
Focuses: | accessibility, javascript | Cc: |
Description
Can we add a keyboard shortcut to the customizer for collapsing/expanding the left panel?
Attachments (10)
Change History (61)
#3
in reply to:
↑ description
@
12 years ago
- Keywords close added
Suggest won't fix due to potential conflicts with assistive technology. Might be a good plugin though.
As pointed out by esmi in #21283:
For that reason my own recommendation would be to not employ keystrokes here.
Loudly seconded! The last time a group of us looked at what keys were available once you removed all AP software keys and took multiple languages into account, there were only about 3 keys left apart from the numerics. Use anything apart from 0-9 and you will hijack someone's AT. Ten numbers don't exactly leave you a lot to play with given that you also need to use them consistently right across the board.
#4
@
12 years ago
I use WordPress an inordinate amount of time :) and I'm a big fan of keyboard shortcuts. In WordPress, comment moderation keyboard shortcuts are opt in (Users → Your Profile → Keyboard Shortcuts), so maybe customizer keyboard shortcuts could be the same.
#5
@
10 years ago
- Focuses accessibility javascript added
- Keywords needs-patch good-first-bug added; close removed
- Milestone changed from Awaiting Review to Future Release
- Summary changed from Add a keyboard shortcut for collapse/expand in the customizer to Add keyboard shortcuts for collapse/expand, panel-back, and close to the Customizer
- Version set to 3.4
In the past two years, we've shifted to using keyboard shortcuts much more prominently, often to aid with keyboard nav/accessibility. @helen mentioned using keyboard shortcuts for Panels and closing the Customizer several times during 4.0 development; we should look at those too.
In fact, I'd suggest that we start by adding esc
to exit a panel (if in a sub-panel -- there's a class added to a container that we can check), and also esc
to close the Customizer, if not in a panel (we now have an AYS to prevent accidental closures when changes are pending).
Once we've implemented that, let's look at whether there's a good option for expanding/collapsing the controls area. Not sure what the shortcut should be, but we should only add it if it's intuitive enough that we don't need to make it a configuration option. Based on @jorbin's note above, there may not be a good option here in terms of accessibility, but I'd be interested in any more updated thoughts from the accessibility team.
Marking as good first bug since this is a fairly isolated request, and should be reasonably straightforward code-wise once the UX is nailed down (and a good place to jump in and start, with additional iterations expected). I'd put the js for it in wp-admin/js/customize-controls.js
, in the less structured part of the file (after the control models).
#6
@
10 years ago
esc
would probably be as far as I'd be comfortable with "hijacking" keyboard shortcuts.
#7
@
10 years ago
I think it would be best, for now, to simply use esc to exit the current panel. It's a standard, well understood keyboard usage. Any other shortcuts would introduce potential problems.
There is the option of defining the entire customizer as having role=application for ARIA, but I'm not sure that's justified; this UI isn't that complex.
#8
@
10 years ago
Hi There!
How about if the user taps escape, and the customizer is expanded, with a subtab expanded, we slide up the expanded tab. if they tap escape again, or when there are no sub tabs expanded, the customizer sidebar collapses. The slide up and collapse actions don't clear out unsaved changes, so we don't need to show an AYS message. I don't think escape should cause any destructive action - just hide the modal that's expanded. I've attached a patch to the customize-controls.js file to demo this functionality.
Thoughts?
-Bryan
#10
@
10 years ago
I like the sound of that; it's simple and intuitive, and shouldn't break any expected behavior for AT.
#13
@
10 years ago
Definitely. @rianrietveld, can you get this in the testing docket for the next round of tests? (Rian coordinates the accessibility user testing group.)
This ticket was mentioned in Slack in #core-customize by afercia. View the logs.
10 years ago
#16
@
10 years ago
Asked @celloexpressions if we should still test this but seems some relevant changes will be in soon: https://wordpress.slack.com/archives/core-customize/p1423966725000697
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
10 years ago
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
10 years ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
10 years ago
#22
@
10 years ago
- Keywords needs-refresh added
@purcebr Would you like to refresh your patch for [32649]?
#23
in reply to:
↑ 19
@
10 years ago
- Owner changed from purcebr to celloexpressions
@celloexpressions, we're one week away from beta1, could you make sure this gets a refreshed patch and committed/punted until then?
#25
@
10 years ago
Thanks @purcebr - I'll review it tomorrow evening and let you know how it's looking. Should be able to make it in but we can always put if it ends up being trickier than expected.
@obenland - will do.
#26
@
10 years ago
@celloexpressions - I've attached the revamped patch. Be sure to try out the feature on both the sections (Site Title and Tagline, Colors, Header Image, Background Image, Static Frontpage on my default install) and the panels (Widgets, Menus) and their sub sections.
Let me know how it looks from your end!
thanks,
Bryan
#27
@
10 years ago
Just wanted to touch base to make sure this was on track, and you've got everything you need from me for the refreshed patch.
thanks!
Bryan
#28
@
10 years ago
- Keywords needs-refresh removed
Patch looks good, although needs a few extra spaces added. Note that we use spaces around either side of conditionals, within functions, etc. For example, if( overlay.hasClass('expanded') ) {
should become if ( overlay.hasClass( 'expanded' ) ) {
.
I can't test the patch right now, can someone confirm that it functions as intended?
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
10 years ago
#30
@
10 years ago
@celloexpressions - thanks for the feedback, attached a new patch. Let me know if you see any other issues!
#31
@
10 years ago
Thanks. One other thing I didn't notice before: please always patch from the root of WordPress, rather than from the file directly. So the path in the patch file should be something like src/wp-admin/js/customize-controls.js
instead of customize-controls.js
.
#32
@
10 years ago
- Keywords commit added
Testing, patch works great. Other than the patch path, this is ready for commit.
This ticket was mentioned in Slack in #core by obenland. View the logs.
10 years ago
#35
@
10 years ago
- Keywords needs-patch added; has-patch commit removed
I don't know about collapsing the sidebar with escape - I had always thought of escape as closing the customizer, not a one way toggle for the sidebar. It's very frustrating when it collapses the sidebar, as you can't quickly expand it again, or at least I didn't find any way to do that. It's also not super useful (as noted in many other places), so let's either make escape close the customizer or not do anything if you're at the top level. Need to get this done in the next couple of hours if we want it for 4.3.
#36
@
10 years ago
@helen - I see what you're saying here. I like that escape performs non-destructive transitions. If an escape at the top level closed the customizer i'd think that would be startling considering the other ways it's set up in this patch. The ESC collapse feature is useful for testing out how the theme would look, but yeah the arrow in the lower left to reexpand is small and easy to miss. And there's no similar key command to reexpand, so it's not super intuitive. (Unless Enter to reexpand? which might be confusing or unworkable for other reasons :) )
I'll reattach a patch that doesn't collapse the customizer at the root level. Let me know if you'd prefer to go the other route instead, and I can get another patch together!
Thanks for the feedback,
Bryan
This ticket was mentioned in Slack in #core-customize by purcebr. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by obenland. View the logs.
10 years ago
#39
@
10 years ago
- Milestone changed from 4.3 to Future Release
Let's look at this again in a future release.
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
9 years ago
#41
@
9 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Future Release to 4.6
22237.diff refreshes @purcebr's latest patch for current trunk.
This is really smooth. esc
closes the current section or panel, and is the only keyboard shortcut added here (it doesn't close or collapse the panel). Also noting some of the initial concerns in the accessibility team chat linked above, the exited section/panel is focused after a section is closed, so you can quickly get back into the section or panel by pressing enter after pressing escape.
This should be ready for commit.
#43
@
9 years ago
agreed, we should be able to get rid of that. At the moment, there's no handler for esc in this view, so it works as expected, but there's no guarantee of that in the future, or that there aren't other handlers loaded to listen for esc presses.
#44
@
9 years ago
Aside: This Esc keyboard shortcut will make a good power user workaround while waiting for #35186 (Put the Customizer "back" button next to the "Close" button)
@celloexpressions Assign to me with commit
when you're confident it's good to go.
#45
@
9 years ago
- Keywords commit added
- Owner changed from celloexpressions to westonruter
- Status changed from assigned to reviewing
I realized that we should preventDefault
when we do close out of a section of a panel, but not otherwise. If, for example, esc
is a shortcut to exit a full screen mode for a browser or something like that, this would prevent that from happening when it triggers section collapses but not when you're at the top level. That's probably the best way to account for anything browsers or devices might be doing with esc
, allowing that action to occur one all levels of ours are complete.
I think 22237.1.diff is ready to commit.
#46
@
9 years ago
- Keywords commit removed
- Owner changed from westonruter to celloexpressions
@celloexpressions take a look at my new patch 22237.2.diff . What do you think about also recognizing Esc when a widget or nav menu item is expanded? This adds support for that and also improves parity between the controls for nav menu items and widgets. I noticed the former lacked the same interface as the latter.
#48
@
9 years ago
- Keywords commit added
- Owner changed from celloexpressions to westonruter
Found a minor issue with menus that's fixed in 22237.3.diff (was still using toggleForm in one place). Should be good to go now.
Related: #21283