fix: escape key navigates back from search, lyrics, favorites, and plugin manager#14
fix: escape key navigates back from search, lyrics, favorites, and plugin manager#14pipe-code wants to merge 4 commits into
Conversation
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the search bar’s Escape-key handler so that it either clears non-empty input or triggers a GO_BACK navigation action when the input is empty or the bar is inactive, ensuring keyboard navigation back from search/results views. Sequence diagram for Escape key handling in SearchBarsequenceDiagram
actor User
participant SearchView
participant SearchBar
participant KeyBinding
participant Store
User->>SearchView: press escape key
SearchView->>KeyBinding: onKeyDown(escape)
KeyBinding->>SearchBar: clearSearch(bypassBlock=true)
alt SearchBar isActive and input not empty
SearchBar->>SearchBar: setInput("")
SearchBar->>SearchView: onInput("")
else input empty or SearchBar inactive
SearchBar->>Store: dispatch(GO_BACK)
Store-->>SearchView: navigate back
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the search functionality's keyboard navigation by refining the behavior of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the SearchBar component by modifying the clearSearch function. The escape key now provides dual functionality: it clears the search input if text is present, or navigates back if the input is already empty. The useCallback dependencies for clearSearch were correctly updated to include input and dispatch to ensure proper behavior. There is no further feedback to provide.
| dispatch({category: 'GO_BACK'}); | ||
| } | ||
| }, [isActive, onInput]); | ||
| }, [isActive, onInput, input, dispatch]); |
There was a problem hiding this comment.
The inclusion of dispatch in the useCallback dependency array for clearSearch is important for robustness. While dispatch from useReducer is stable, the useNavigation hook's context value is memoized based on state and dispatch. As state can change, the dispatch function reference returned by useNavigation might not be stable across renders. Explicitly including it ensures that the clearSearch callback always uses the most current dispatch function, preventing potential stale closure issues or unexpected behavior.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Overview
Files Reviewed (4 files)
Type Safety VerificationAll changes use the properly typed Security CheckNo secrets, API keys, or credentials introduced in this diff. All token/secret references in the codebase are pre-existing patterns for user configuration. Changeset Note |
pipe-code
changed the title
1 participant
What's the issue?
Pressing
Escapein several views either did nothing or only partially worked:Escapeagain (empty input) or pressing it while browsing results did nothing.Escapebinding at all.Esc=Backin the shortcuts bar but never wired it up.What's the fix?
SearchBar.tsx(original fix)The
clearSearchcallback now checks whether the input is empty before deciding what to do:GO_BACKLyricsLayout.tsx,FavoritesList.tsx,PluginsLayout.tsx(expanded fix)Each view now imports
useNavigationand registers an escape binding withbypassBlock: trueso it always fires:For Plugin Manager specifically,
Escapefirst closes the install dialog if it's open, and only dispatchesGO_BACKif already on the list view.The root cause across all views is the same:
SearchBarregisters its escape binding withbypassBlock: true, intercepting the key before the global handler can fire. Views without their own escape binding were simply left without any back navigation on keyboard.Files changed
source/components/search/SearchBar.tsxGO_BACKfallback when input is emptysource/components/layouts/LyricsLayout.tsxGO_BACKbindingsource/components/favorites/FavoritesList.tsxGO_BACKbindingsource/components/layouts/PluginsLayout.tsxGO_BACK(or close install dialog) bindingSummary by Sourcery
Bug Fixes: