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

Allow shortcuts to work inside input fields with flag #1

Closed
zshall opened this issue Apr 7, 2020 · 4 comments
Closed

Allow shortcuts to work inside input fields with flag #1

zshall opened this issue Apr 7, 2020 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@zshall
Copy link
Contributor

zshall commented Apr 7, 2020

A common use case for keyboard shortcuts is to work while input or textarea elements are focused. Keymaster's default filter excludes these elements when firing shortcuts.

Searching around the Keymaster repo, I've found a few pages discussing this:

https://github.com/madrobby/keymaster#filter-key-presses
madrobby/keymaster#43
madrobby/keymaster#165

Issue 43 is closest to what I was thinking about. You'd have a flag, such as @combo("shortcut", true). When this is set, the shortcut would run anywhere, including inputs. If the flag wasn't set, the shortcut would be filtered out of inputs using the default filter.

Another idea would be to support custom filtering functions. Imagine something like @combo('shortcuts', this.filter) where this.filter is a function in the current class that would only trigger the keyboard shortcut if it returned true.

I'd be willing to help tackle this, but I'd like to figure out the best way before going about it.

One approach would be to override Keymaster's built-in filtering with a new filtering system.

key.filter = (ev) => {
    return true;
}

Disabling the regular filter, and when registering a shortcut, doing the equivalent of this:

  [where the plugin is called]() {
    key('ctrl+f, command+f', e => {
      if (flag || (!flag && [we're not in an input or textarea)]) {
        if (true !== this.findIt(e)) e.preventDefault();
      }
    })
  }

  @combo('ctrl+f, command+f', true)
  findIt(e) {
    console.log('findIt');
  }

Any additional thoughts would be appreciated!

@3cp
Copy link
Member

3cp commented Apr 7, 2020

Sounds great! I also had same problem of firing short-cut from input.

@3cp
Copy link
Member

3cp commented Apr 7, 2020

Currently I have keydown handler on the input itself. Like this:

  keyDownInFilter(e) {
    if (e.key === 'ArrowUp') { // up
      e.target.blur();
      this.selectPrevious();
      return false;
    } else if (e.key === 'ArrowDown') { // down
      e.target.blur();
      this.selectNext();
      return false;
    } else if (e.key === 'Enter') { // return
      this.open(this.selectedIdx);
      return false;
    }

    return true;
  }

It does things for ArrowUp, ArrowDown, and Enter, otherwise return true (an aurelia convention) to NOT preventDefault so input can consume other keys.

What you proposed is hard to implement. The issue is that input consumes all keyboard events without sending them up to document root where keymaster listens global keyboard events.

So technically, the global event handler created by keymaster cannot do anything about input. Some handlers on input elements themselves are probably needed to help.

I will do some experiment when I got time.

@3cp 3cp self-assigned this Apr 7, 2020
@3cp 3cp added the enhancement New feature or request label Apr 7, 2020
@3cp
Copy link
Member

3cp commented Apr 7, 2020

Be my guest if you want to work on it! I have no idea what approach is better or could work. This's a small lib, so doing experiments are considerably cheap.

zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
@zshall
Copy link
Contributor Author

zshall commented Apr 8, 2020

master...zshall:#1---Input-Fields - I've done a bit of experimenting myself and this is what I've come up with so far:

  • Disable the default filter that Keymaster uses, processing key commands on all keypresses.
  • Changing the signature of @combo() to declare a list of shortcuts as a single parameter.
  • Adding a second parameter, runInsideInputs, a flag to determine whether we should filter or not.
  • Adding an if block inside the Keymaster handler to return true if we should be filtering inside inputs, and the default filter detects that we are currently in an input.

I added examples of both to the dev-app.

Please let me know your thoughts on this implementation!

zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 8, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 9, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 9, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 9, 2020
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 9, 2020
When we dispatch the key event, use `document.activeElement` as the event target.
zshall added a commit to zshall/aurelia-combo that referenced this issue Apr 9, 2020
@3cp 3cp closed this as completed Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants