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

Keyboard shortcut registry #90515

Open
myasonik opened this issue Feb 5, 2021 · 8 comments
Open

Keyboard shortcut registry #90515

myasonik opened this issue Feb 5, 2021 · 8 comments
Labels
enhancement New value added to drive a business result old Used to help sort old issues on GH Projects which don't support the Created search term. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@myasonik
Copy link
Contributor

myasonik commented Feb 5, 2021

Summary

Several teams have implemented their own keyboard shortcuts for various bits of UI throughout Kibana. This, on top of the keyboard shortcuts that EUI ships, are bound to start conflicting at some point.

Managing discovery of keyboard shortcuts also poses a challenge that no one has yet undertaken nor have the accessibility issues of shipping arbitrary keyboard shortcuts been tackled.

Goal

Provide a way for Kibana teams and EUI components to register their keyboard shortcuts to improve discoverability and allow for a central management of them from both a developer and a user perspective.

Do people want this?

Not including issues of people complaining about our tab order not being what they want or expect (which strong keyboard shortcuts could help mitigate):

Functionality & features

MVP

  • A service for developers to register their keyboard shortcuts to
    • Can manage uniqueness: Critical on a per-page basis but also good to know across different pages
  • A UI for users to view all shortcuts available on the current page
    • Available in the help context menu and under a ? keyboard shortcut
  • A system for EUI components to expose and register their shortcuts

Phase 2

  • Allow users to edit and disable keyboard shortcuts through the UI
    • Can start off as a setting saved in local storage and can eventually migrate to a user setting when that is available. (Doesn’t make sense as a space setting because these customizations will often be driven by unique reasons such as working with specific assistive tech or power-user specific workflows.)
    • This unlocks the capability to provide single key shortcuts (WCAG 2.1.4, any of which would right now pose as an WCAG A failure)

Future work

  • Allow users to see and edit all keyboard shortcuts throughout Kibana
    • Can live somewhere in advanced settings (Use case is more centered around admin changing something for everyone because of collisions with other software or a user wanting to disable all keyboard shortcuts)
  • Allow users to set keyboard shortcuts for commands registered without a shortcut
    • For example, could register “Create a new table visualization” as an action without a default keyboard shortcut. A user may find it useful to add this as a shortcut if it is a common action for them.
    • Ties in with future work of Global Search (anything that is available in global search is a good candidate to being available at a keyboard shortcut)

Accessibility notes

  • Shortcuts fall in 3 categories: Navigation (move focus to a specific element), Activation (perform a specific action that does not have focus and may or maynot be visible), or Both
  • Keyboard shortcuts should never be the only means of performing an action and should not replace appropriate focus order
  • Avoid using any modifier keys (Meta, Alt/Option, Caps lock, Insert, Scroll lock)
  • Avoid differentiating between capital and lowercase letters
  • There is an HTML attribute called accesskey — unfortunately, it largely doesn’t do anything and is buggy in places where it should do something so it’s best to avoid it entirely
  • Though not super well supported, we can enhance keyboard shortcuts by adding aria-keyshortcuts. The largest difficulty here will be updating these values when customized by users

Not addressed

  1. Should CoreUI prescribe specific keyboard shortcuts or only create a mechanism for other teams to register theirs?
    • Common keyboard shortcuts seem to be:
      g+ Go to...
      ? Help
      j/k Next/Previous
  2. How to handle localization? (Do we want to at all?)
  3. Does global search become an all purpose command palette or do we keep these UIs separate?
  4. How do we differentiate component specific shortcuts (such as code editors, EUI’s upcoming markdown editor, and other EUI components), page specific shortcuts, and global shortcuts?

Examples

  1. Gmail has an extensive keyboard shortcut page that’s always available
    Screen Shot 2020-02-21 at 18 08 50

    a. And allows for individually configuring each option, giving an alternative, and resetting all of it back to defaults
    Screen Shot 2020-02-21 at 18 15 24

  2. Google Calendar has a more modern design of Gmail’s implementation (but the only control is a on/off toggle). Twitter, Feedly, Google Keep, Trello — all have similar designs and implementations.
    Screen Shot 2020-02-21 at 18 13 26

  3. GitHub shows you the keyboard settings available on the page you’re on and links out to all keyboard settings (and has no controls for them)
    Screen Shot 2020-02-21 at 18 21 22

@myasonik myasonik added Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result labels Feb 5, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-design (Team:Kibana-Design)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

I don't foresee any major technical issue registering global or per-app shortcuts.

The tricky part here will be to find the correct API to let plugins register per-page shortcuts, as an application 'page' is a concept unknown from core. The plugins will probably have to manually refresh/update the shortcuts when the user transition from a page to another

@myasonik
Copy link
Contributor Author

All our page transitions go through a single function, don't they? Could core use that transition as a signal to update the active shortcuts on a page?

Then shortcuts could hopefully be global, tied to an app, or tied to a URL.

But another difficulty would be what happens with certain EUI components registering shortcuts? Especially on pages like Dashboard where a lot of stuff is user driven, we'd need a way to register a shortcut from a rendered component, not from something like the plugin registry.

@pgayvallet
Copy link
Contributor

pgayvallet commented Feb 12, 2021

All our page transitions go through a single function, don't they?

No, they don't necessarily go through application.navigateToApp. Applications can also navigate via their scoped history (history.push), which is what react-router's Link is doing.

We could eventually listen to the history from core and try to 'deduce' the page from here, but that could be harder than it sounds, for multiple reasons.

Also, I doubt that, in term of developer experience, registering everything during a plugin's startup would be the easiest approach. Most shortcuts are usually bounds to page-local handlers, meaning that registering everything for all their pages in advance would result in a way harder implementation for the consumers.

I really think 'per-page' shortcut could be dynamically added by the pages components.

e.g

const MyPage: FC = ({ params }: { params: AppMountParameters }) => {

   useEffect(() => {
      const unregister = params.shortcuts.register(myShortcuts);
      return () => unregister();
   }, [])
}

The example is exposing the local shortcut registry on AppMountParameters, I think it would be better than exposing it from a service, to restrict local shortcut registration to a given application's mount instead of the whole plugin. Global shortcuts would be registered using coreStart.shortcuts.registerGlobal()

This also has the upside that the shortcuts are not bound to the concept of 'pages', but of components instead. Nested routes could have multiple nested components adding potential shortcuts depending on the state of the app.

One downside is that we will not be able to detect shortcut conflicts during core's setup/start phase, and only when a potentially conflicting app is mounted.

@myasonik
Copy link
Contributor Author

One downside is that we will not be able to detect shortcut conflicts during core's setup/start phase, and only when a potentially conflicting app is mounted.

I think that's a reasonable downside at least until it becomes a problem which could still be a long way away unless people start adding these all over very suddenly.

@majagrubic majagrubic added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Feb 1, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@bhavyarm
Copy link
Contributor

cc @1Copenut

@petrklapka petrklapka added the old Used to help sort old issues on GH Projects which don't support the Created search term. label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result old Used to help sort old issues on GH Projects which don't support the Created search term. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

No branches or pull requests

6 participants