Skip to content

Commit

Permalink
Deprecate shortcut in favor of shortcutLabel and ariaKeyShortcuts
Browse files Browse the repository at this point in the history
Don't try to interpret `shortcut` as `aria-keyshortcuts`, as it will most often not work.
`aria-keyshortcuts` needs "Control" spelled out, and for macOS "Meta" for the command key, which you probably don't want to call it in your menus!
  • Loading branch information
1j01 committed Dec 19, 2021
1 parent 2534fd8 commit 92541cb
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
17 changes: 10 additions & 7 deletions MenuBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,12 +534,9 @@ function MenuBar(menus) {
// but it is read in translated labels like "새로 만들기 (&N)".
// The AccessKeys.remove() is so it doesn't announce an ampersand from the access key.
item_el.setAttribute("aria-label", AccessKeys.remove(item.label || item.item));
// include the shortcut semantically; if you want to display the shortcut differently than aria-keyshortcuts syntax,
// provide both ariaKeyShortcuts and shortcutLabel (old API: shortcut)
// @TODO: why am I doing `|| item.shortcutLabel` here? isn't that kinda against the point?
// if you're providing just shortcutLabel, maybe you're indicating that it's not aria-keyshortcuts syntax,
// like for a non-keyboard shortcut? "Swipe Up", "Blink Twice", "Clap", "Mouse Wheel", etc.
item_el.setAttribute("aria-keyshortcuts", item.ariaKeyShortcuts || item.shortcut || item.shortcutLabel);
// Include the shortcut semantically.
// TODO: automatically include the access key, while the specific menu (or menu bar) is focused, and update docs to note this
item_el.setAttribute("aria-keyshortcuts", item.ariaKeyShortcuts || "");

if (item.description) {
item_el.setAttribute("aria-description", item.description);
Expand All @@ -555,7 +552,13 @@ function MenuBar(menus) {
item_el.appendChild(submenu_area_el);

label_el.appendChild(AccessKeys.toFragment(item.label || item.item));
shortcut_el.textContent = item.shortcut;

if (item.shortcutLabel) {
shortcut_el.textContent = item.shortcutLabel;
} else if (item.shortcut) {
shortcut_el.textContent = item.shortcut;
console.warn("Menu item option `shortcut` is deprecated; use `shortcutLabel` instead (and ideally provide `ariaKeyShortcuts` as well)");
}

menu_popup_el.addEventListener("update", () => {
// item_el.disabled = is_disabled(item); // doesn't work, probably because it's a <tr>
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ menubar.element.addEventListener("default-info", (event)=> {
Menu item specifications are either `MENU_DIVIDER` - a constant indicating a horizontal rule, or a radio group specification, or an object with the following properties:
* `label`: a label for the item; ampersands define [access keys](#access-keys) (to use a literal ampersand, use `&&`)
* `shortcut` (optional): a keyboard shortcut for the item, like "Ctrl+A"; this is not functionally implemented, you'll need to listen for the shortcut yourself!
* `shortcutLabel` (optional): a keyboard shortcut to show for the item, like "Ctrl+A" (Note: you need to listen for the shortcut yourself, unlike access keys)
* `ariaKeyShortcuts` (optional): [`aria-keyshortcuts`](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-keyshortcuts) for the item, like "Control+A Meta+A", for screen readers. "Ctrl" is not valid (you must spell it out), and it's best to provide an alternative for macOS, usually with the equivalent Command key, using "Meta" (and `event.metaKey`).
* `action` (optional): a function to execute when the item is clicked (can only specify either `action` or `checkbox`)
* `checkbox` (optional): an object specifying that this item should behave as a checkbox.
Property `check` of this object should be a function that *checks* if the item should be checked or not, returning `true` for checked and `false` for unchecked. What a cutesy name.
Expand Down

0 comments on commit 92541cb

Please sign in to comment.