Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Enable search results keyboard navigation by default#45890

Merged
novoselrok merged 3 commits into
mainfrom
rn/enable-search-results-keyboard-nav-by-default
Dec 22, 2022
Merged

Enable search results keyboard navigation by default#45890
novoselrok merged 3 commits into
mainfrom
rn/enable-search-results-keyboard-nav-by-default

Conversation

@novoselrok

@novoselrok novoselrok commented Dec 21, 2022

Copy link
Copy Markdown
Contributor

Enables search results keyboard navigation by default

Test plan

Use Arrow Up/Down keys to navigate between search results, Arrow Left/Right to collapse and expand file matches, Enter to open the search result in the current tab, Ctrl/Cmd+Enter to open the result in a separate tab, / to refocus the search input, and Ctrl/Cmd+Arrow Down to jump from the search input to the first result. Arrow Left/Down/Up/Right in previous examples can be substituted with h/j/k/l for Vim-style bindings.

App preview:

Check out the client app preview documentation to learn more.

@novoselrok novoselrok requested a review from olafurpg December 21, 2022 08:26
@cla-bot cla-bot Bot added the cla-signed label Dec 21, 2022
@sourcegraph-bot

sourcegraph-bot commented Dec 21, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff b3fe0bc...696d417.

Notify File(s)
@fkling client/search-ui/src/components/CommitSearchResult.tsx
client/search-ui/src/components/FileContentSearchResult.tsx
client/search-ui/src/components/RepoFileLink.tsx
client/search-ui/src/components/RepoSearchResult.tsx
client/search-ui/src/components/SymbolSearchResult.tsx
client/search-ui/src/results/useSearchResultsKeyboardNavigation.ts
client/web/src/search/results/StreamingSearchResults.tsx
@limitedmage client/search-ui/src/components/CommitSearchResult.tsx
client/search-ui/src/components/FileContentSearchResult.tsx
client/search-ui/src/components/RepoFileLink.tsx
client/search-ui/src/components/RepoSearchResult.tsx
client/search-ui/src/components/SymbolSearchResult.tsx
client/search-ui/src/results/useSearchResultsKeyboardNavigation.ts
client/web/src/search/results/StreamingSearchResults.tsx

@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Dec 21, 2022

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
-0.00% (-0.07 kb) -0.00% (-0.71 kb) -0.01% (-0.64 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 696d417 and b3fe0bc or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@olafurpg olafurpg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍🏻 I noticed two issues that we can fix separately or in this PR.

CleanShot 2022-12-21 at 13 01 39

  • The search bar got auto-focused after opening a search result with Enter making it not possible to go back in browser history with Cmd+ArrowLeft
  • After going back in browser history from the blob page to the search results, the top result was focused when I expected the focused item to be persiste result I opened to be focused.

@novoselrok

Copy link
Copy Markdown
Contributor Author

LGTM 👍🏻 I noticed two issues that we can fix separately or in this PR.

  • The search bar got auto-focused after opening a search result with Enter making it not possible to go back in browser history with Cmd+ArrowLeft
  • After going back in browser history from the blob page to the search results, the top result was focused when I expected the focused item to be persiste result I opened to be focused.

Yep, both issues are still in there from the original PR. I looked into it briefly, but there seems to be a deeper problem with search input focusing that I probably won't be able to fix in this PR. Will probably require a separate PR.

@novoselrok novoselrok merged commit 8bfa5f9 into main Dec 22, 2022
@novoselrok novoselrok deleted the rn/enable-search-results-keyboard-nav-by-default branch December 22, 2022 08:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants