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

Enhance the behaviour of Esc in the Emoji-picker popup #1491

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Niloth-p
Copy link
Collaborator

@Niloth-p Niloth-p commented Apr 27, 2024

What does this PR do, and why?

Previously, pressing Esc anytime when the emoji picker is in use will exit the popup.

Now, different behaviours are supported depending on the state.

1. Escaping from a non-empty searchbox clears it (This is useful since some other editors support using the Ctrl l shortcut to clear the editor, but there's none for the emoji-picker popup).
2. Escaping from an empty searchbox closes the popup.

3. Escaping from the emoji list exits you out of the popup, if the selected emoji button was toggled after the last search (Allow users to exit the popup as soon as they have made some changes (either newly selected or newly deselected)).
4. Otherwise, it takes you back to the searchbox and retains the last search text. (the user has not made changes or is still navigating in the emoji picker, so allow them to return to the search box without exiting the popup)

External discussion & connections

How did you test this?

  • Manually - Behavioral changes
  • Manually - Visual changes
  • Adapting existing automated tests
  • Adding automated tests for new behavior (or missing tests)
  • Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

  • It is a minimal coherent idea
  • It has a commit summary following the documented style (title & body)
  • It has a commit summary describing the motivation and reasoning for the change
  • It individually passes linting and tests
  • It contains test additions for any new behavior
  • It flows clearly from a previous branch commit, and/or prepares for the next commit

Previously, pressing Esc anytime when the emoji picker
is in use will exit the popup.

Now, different behaviours are supported depending on the state.

Escaping from a non-empty searchbox clears it.
Escaping from an empty searchbox closes the popup.

Escaping from the emoji list exits you out of the popup,
if the selected emoji button was toggled after the last search.
Otherwise, it takes you back to the searchbox
and retains the last search text.
@zulipbot zulipbot added the size: S [Automatic label added by zulipbot] label Apr 27, 2024
@neiljp neiljp added the further discussion required Discuss this on #zulip-terminal on chat.zulip.org label May 9, 2024
@neiljp
Copy link
Collaborator

neiljp commented May 9, 2024

#1492 reduced the need for point 1, but otherwise this is still under discussion in the stream.

I suspect we won't take this particular approach, but it is useful for discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
further discussion required Discuss this on #zulip-terminal on chat.zulip.org size: S [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants