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 auto hide search popup #592

Merged

Conversation

toasted-nutbread
Copy link

Fixes #580.

The issue was that the _search function wasn't invoked when there was no text source, which resulted in the searched event never being invoked for that situation. This change happened during the type annotations conversion due to some confusion about how nulls should be handled.

In this change, I've done a little bit of refactoring to make this less likely to occur. The searched event has been split into three different events to clarify the intent:

  • searchSuccess - happens when the search was successful and should have something displayed in the popup. This could be dictionary results, or the query parser.
  • searchEmpty - no text source was present, or no search results were found.
  • searchError - some error occurred.

@toasted-nutbread toasted-nutbread requested a review from a team as a code owner January 30, 2024 00:44
Copy link

github-actions bot commented Jan 30, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

djahandarie
djahandarie previously approved these changes Jan 31, 2024
Copy link
Collaborator

@djahandarie djahandarie left a comment

Choose a reason for hiding this comment

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

Thanks! Maybe it'd be good to have an end-to-end test for something related to this in the playwright suite?

Also, merge conflict.

@djahandarie djahandarie added this pull request to the merge queue Feb 3, 2024
Merged via the queue into yomidevs:master with commit 6dc7e90 Feb 3, 2024
5 checks passed
@djahandarie djahandarie added the kind/bug The issue or PR is regarding a bug label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-hide search popup does not work
2 participants