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

Fix search was not stopped when leaving focus with esc key #1240

Merged
merged 2 commits into from
Dec 21, 2024

Conversation

zoglo
Copy link
Contributor

@zoglo zoglo commented Dec 21, 2024

Description

This PR aims to refresh the list when clicking escape if a search is active.
There currently is a bug where you'd search for something and want to focus out of it when pressing escape - when this happens, you'll lose focus completely and there will not be any items left with a blank search input.

This can be reproduced in the current demo when searching for something and pressing escape - the choices will be gone and only appear again if you type something and then delete it again:
https://choices-js.github.io/Choices/

-> See Screenshot below

Screenshots (if appropriate)

image

Types of changes

  • Chore (tooling change or documentation change)
  • Refactor (non-breaking change which maintains existing functionality)
  • 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)

Checklist

  • My code follows the code style of this project.
  • I have added new tests for the bug I fixed/the new feature I added.
  • I have modified existing tests for the bug I fixed/the new feature I added.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@Xon
Copy link
Collaborator

Xon commented Dec 21, 2024

this.refresh(false, false, true); is probably the wrong function to call here, as it it rebuilds the internal state from the raw <select>'s options list which is much more heavy weight than required.

I think swapping for this._stopSearch(); will make this work as expected

@Xon Xon added bugfix Pull request that fixes an existing bug changes required Pull request requires changes before it can be merged labels Dec 21, 2024
@Xon Xon removed the changes required Pull request requires changes before it can be merged label Dec 21, 2024
@Xon Xon changed the title Refresh available options when leaving focus with esc key Fix search was not stopped when leaving focus with esc key Dec 21, 2024
@zoglo
Copy link
Contributor Author

zoglo commented Dec 21, 2024

this.refresh(false, false, true); is probably the wrong function to call here, as it it rebuilds the internal state from the raw <select>'s options list which is much more heavy weight than required.

I think swapping for this._stopSearch(); will make this work as expected

Yeah, just tested it and it works as espected - good catch, didn't see that method :')

@Xon Xon merged commit 1876714 into Choices-js:main Dec 21, 2024
8 of 10 checks passed
@zoglo zoglo deleted the fix/escape-key-options branch December 21, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request that fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants