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

typeahead: Properly handle custom triggers. #14246

Open
aero31aero opened this issue Mar 19, 2020 · 0 comments · May be fixed by #14247
Open

typeahead: Properly handle custom triggers. #14246

aero31aero opened this issue Mar 19, 2020 · 0 comments · May be fixed by #14247

Comments

@aero31aero
Copy link
Member

We have a kind of elusive bug in our custom typeahead triggers that is applicable only for characters that require two keypresses to type and is only exhibited usually when you're typing fast enough.

It just so happens that the only custom trigger we currently use is > for triggering the stream-topic typeahead, and pressing it is a combination of two keys: Shift + ..

Here's how the current code looks:

keyup: (e) {
  if (trigger_selection(e)) {
    select();
  }
}

Now, inside the trigger selection function, if we ask for e.keyCode, we always get 190 which is the keycode for .. Thus, we need to use e.key which gives us the intended value >... unless, the person release the shift key first and then releases the . key. Both these keyup events fail the check because we get e.key = '' and e.key = '.' respectively.

To fix this reliably while preserving the similar behavior (of taking action on keyup), here's a proposed solution:

let result = false;

keypress: (e) {
  if (trigger_selection(e)) {
    result = true;
  }
}

keyup: (e) {
  if (result) {
    result = false;
    select();
  }
}
aero31aero added a commit to aero31aero/zulip that referenced this issue Mar 19, 2020
@aero31aero aero31aero linked a pull request Mar 19, 2020 that will close this issue
aero31aero added a commit to aero31aero/zulip that referenced this issue Mar 19, 2020
aero31aero added a commit to aero31aero/zulip that referenced this issue Mar 20, 2020
aero31aero added a commit to aero31aero/zulip that referenced this issue Mar 20, 2020
aero31aero added a commit to aero31aero/zulip that referenced this issue Apr 20, 2020
aero31aero added a commit to aero31aero/zulip that referenced this issue Apr 20, 2020
We switch to using keypress event for custom triggers because keyup can be
tricky to handle for combo keys. If you press 'shift' + '.', you get >
However, we never get the keyup event for '>'. Instead, we get one
for '.' and one for 'shift'. If you check the e.key value, depending
on whether shift was released first or later, we can get either '.' or '>'.

Fixes zulip#14246.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants