Fix keyboard search when left pane is narrow #6303
Merged
Conversation
This file contains 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
veekas
force-pushed
the
bug/keyboard-search-with-narrow-left-pane-6281
branch
from
28bcd47
to
4156ce8
Compare
This PR fixes Cmd+f/Ctrl+f functionality in the left pane. Using a keyboard to search across all conversations should now automatically change the left pane width to accommodate the search input component. Resolves: signalapp#6281
veekas
force-pushed
the
bug/keyboard-search-with-narrow-left-pane-6281
branch
from
4156ce8
to
0100866
Compare
josh-signal
reviewed
Feb 22, 2023
| (state: SearchStateType): boolean | undefined => state.globalSearch | ||
| ); | ||
|
|
||
| export const getIsSearchingGlobally = createSelector( |
There was a problem hiding this comment.
small nitpick but we probably don't need the above function we can just coerce into a Boolean right here.
There was a problem hiding this comment.
Makes sense. Updated!
josh-signal
approved these changes
Feb 22, 2023
|
Awesome thanks, this will be in the next beta release |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
First time contributor checklist:
Contributor checklist:
mainbranchyarn readyrun passes successfully (more about tests here)Description
Fixes #6281
This PR fixes Cmd+f/Ctrl+f functionality in the left pane. Using a keyboard to search across all
conversations should now automatically change the left pane width to accommodate the search input component.
I made an assumption that when a user searches globally, the behavior of the left pane should be similar to when a user searches within a single conversation.
To do so, I chose to create a new value in state called
globalSearch. I'm not super happy with the name, but I don't have any better ideas. Another approach that seems to work and avoids adding a new value in state would be to assign any string tosearchConversationIdthat doesn't match a real conversation, e.g.searchConversationId: 'global', but that seemed less clear than having a dedicated variable.keyboard.search.mov
Other thoughts/bugs related to search
Although these are out-of-scope for this PR, I noticed a few things while working on this issue that I wanted to capture. I'll break these out into new issues later, assuming none of the Signal maintainers have strong feelings against the proposals.
LeftPaneInboxHelperSEARCH_CLEARis getting called twice in a row when the user stops searching. I think it's getting called in both theonBlurand theonClearhandlers inLeftPaneSearchInput.tsx(lines 101 and 118), but that should be validated.