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

Dev/search click escape #537

Merged
merged 3 commits into from
Aug 2, 2022
Merged

Dev/search click escape #537

merged 3 commits into from
Aug 2, 2022

Conversation

HenrikeW
Copy link
Contributor

@HenrikeW HenrikeW commented Aug 2, 2022

Related issue(s) and PR(s)

When having started an advanced search it's now possible to close the appearing list of search results by clicking outside of it or pressing Escape.
(Pressing Escape was possible before, but only as long focus stayed on the input field. If the user continued tabbing, e.g. to scroll through the list of suggestions, it wasn't possible to use the Escape key to close the list. Now it is.)

The hook could be reused with any DOM element and any desired effect.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

List of changes made

  • the hook is defined in a reusable way
  • it's used in the QuickAdd component

Definition of Done checklist

  • I have made an effort making the commit history understandable
  • I have performed a self-review of my own code and commented any hard-to-understand areas
  • Tests and lint/format validations are passing
  • My changes generate no new warnings

Copy link
Contributor

@jonandernovella jonandernovella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previously created event listener should be removed.

onKeyUp={(e) => onEscapeInput(e)}

@jonandernovella jonandernovella self-requested a review August 2, 2022 15:02
Copy link
Contributor

@jonandernovella jonandernovella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the duplicated event listener. It works as expected!

@jonandernovella jonandernovella self-requested a review August 2, 2022 15:03
@jonandernovella jonandernovella merged commit f33c74f into develop Aug 2, 2022
@jonandernovella jonandernovella deleted the dev/search-click-escape branch August 2, 2022 15:03
@jonandernovella jonandernovella mentioned this pull request Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants