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

search pills: Backspace should remove a search pill with typeahead open. #10092

Closed
wants to merge 1 commit into from

Conversation

shubham-padia
Copy link
Member

Partially fixes #10026.
Typeaheads stopped propogation of keydown and keyup events for any
key except tab and enter. If stopAdvance was true even tab and enter
were not allowed.
advanceKeyCodes option was added to typeahead which allowed to specify
key codes for which propogation of keydown and keyup events should not
stop. advanceKeyCodes does not respect the stopAdvance option.
As the backspace key code is added to advanceKeyCodes in search.js,
the backspace key deletes pill on pressing backspace if input is empty
or only consists of spaces.

I'm not sure why the all the key codes except tab and enter aren't allowed.
Another approach would have been to allow backspace for all typeaheads without adding the advanceKeyCodes option, but I figured it would safer to allow it only for the search typeahead.

peek 2018-07-28 11-13

@zulipbot
Copy link
Member

Hello @zulip/server-search members, this pull request was labeled with the "area: search" label, so you may want to check it out!

@shubham-padia
Copy link
Member Author

shubham-padia commented Jul 28, 2018

(test failures are due to #10091)

Partially fixes zulip#10026.
Typeaheads stopped propogation of keydown and keyup events for any
key except tab and enter. If stopAdvance was true even tab and enter
were not allowed.
advanceKeyCodes option was added to typeahead which allowed to specify
key codes for which propogation of keydown and keyup events should not
stop. advanceKeyCodes does not respect the stopAdvance option.
As the backspace key code is added to advanceKeyCodes in search.js,
the backspace key deletes pill on pressing backspace if input is empty
or only consists of spaces.
@timabbott
Copy link
Sponsor Member

Merged, after fixing the trailing whitespace in bootstrap.js and changing the commit message to not accidentally auto-close the issue. See 28589c5.

Thanks @shubham-padia!

@timabbott timabbott closed this Jul 30, 2018
evykassirer added a commit to evykassirer/zulip that referenced this pull request Mar 7, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
evykassirer added a commit to evykassirer/zulip that referenced this pull request Mar 10, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
evykassirer added a commit to evykassirer/zulip that referenced this pull request Mar 10, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
evykassirer added a commit to evykassirer/zulip that referenced this pull request Mar 11, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
evykassirer added a commit to evykassirer/zulip that referenced this pull request Mar 11, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
timabbott pushed a commit that referenced this pull request Mar 11, 2024
The `advanceKeyCodes` option was introduced in #10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
Shgit29 pushed a commit to Shgit29/zulip that referenced this pull request Mar 13, 2024
The `advanceKeyCodes` option was introduced in zulip#10092.
It included a buggy check for `$.inArray` that returned
a falsey value only when the keycode was the first
element of `advanceKeyCodes`.

Because the only instance of `advanceKeyCodes` right now
is for search and contains only one element, this change
is functionally equivalent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Follow up issues for search pills
3 participants