Connect: Improve focus management in search bar#24665
Merged
ravicious merged 1 commit intoravicious/error-handlingfrom Apr 17, 2023
Merged
Connect: Improve focus management in search bar#24665ravicious merged 1 commit intoravicious/error-handlingfrom
ravicious merged 1 commit intoravicious/error-handlingfrom
Conversation
ravicious
commented
Apr 17, 2023
Comment on lines
128
to
129
Member
Author
There was a problem hiding this comment.
I couldn't find a way to write a test for this part. I think it has to do with how the browser and React interact together so it might be hard to reproduce it in a non-browser environment like our tests.
ravicious
force-pushed
the
ravicious/search-focus
branch
from
2319ff0 to
8df8e93
Compare
gzdunek
approved these changes
Apr 17, 2023
ryanclark
approved these changes
Apr 17, 2023
ravicious
added a commit
that referenced
this pull request
Apr 17, 2023
* Move tshd test helpers to a better location * Support passing no props to tshd test helpers * Refactor ResourcesService getServerByHostname tests * Move pluralize to shared package * SearchContext: Rename `opened` to `isOpen` * ActionPicker story: Show auxiliary items in a separate column * ActionPicker: Split getClusterName into two functions getClusterName used to not return the name of the cluster if there's only a single cluster present. Some places needed to get the cluster name no matter what, such as the modal with resource errors that will be added to ActionPicker. * Refactor resource search to use Promise.allSettled * useSearchAttempts has been renamed to useActionAttempts * useActionAttempts returns resourceSearchAttempt in order to supply errors from ResourcesService.searchResources to ActionPicker. * SearchContext: Implement lockOpen We'll want to display error details in a modal. While the user interacts with the modal, we don't want to close the search bar and reset the results. So instead, we are going to force the search bar to stay open until the user closes the modal. This will use the lockOpen function from this commit. * Add modal for showing resource search errors * Refactor mockedSearchContext to not be a top-level mutable var * Show an item in search bar with resource search errors * ResourceSearchError: Add instanceof check to tests, include clusterUri in message * Make isLockedOpen into a ref * Use table tests for lockOpen tests * Revert "Make isLockedOpen into a ref" This reverts commit 07f4206. * Move capitalization to ResourceSearchError.messageWithClusterName * ResourceSearchError: Use `public` in constructor * Connect: Improve focus management in search bar (#24665)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes it so that after you open the resource search errors modal and close it using a mouse, the focus is brought back to the search input.
Arguably, just the call in
lockOpenwould have been enough but in theory the focus could be lost through other means (like a right click on a search result), so I left the focus calls inopentoo.To reproduce the issue on the previous version of the search bar: