-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[W4Checklist] [Due for payment 2025-05-20] [$250] Cmd+K Navigation Fails to Open Chat After Desktop Update #60709
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
Comments
Triggered auto assignment to @strepanier03 ( |
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Screen.Recording.2025-04-23.at.4.04.44.PM.movAble to repro this. This happens only once when I open the App. |
🚨 Edited by proposal-police: This proposal was edited at 2025-04-23 12:25:46 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Cmd+K Navigation Fails to Open Chat After Desktop Update What is the root cause of that problem?In the #56948 We updated SearchAutocompleteList, we have App/src/components/Search/SearchAutocompleteList.tsx Lines 600 to 601 in 8a9ecc6
App/src/components/SelectionList/BaseSelectionList.tsx Lines 822 to 828 in 806e231
What changes do you think we should make in order to solve the problem?We have two options: Option 1: App/src/components/Search/SearchAutocompleteList.tsx Lines 130 to 138 in 8a9ecc6
shouldSubscribeToArrowKeyEvents = true Option 2: App/src/components/Search/SearchRouter/SearchRouter.tsx Lines 348 to 350 in 8a9ecc6
<SearchAutocompleteList
shouldSubscribeToArrowKeyEvents We can check other places, like this one, to see if a similar issue exists. What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?I think automated tests are not necessary in this case since we're making What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
🚨 Edited by proposal-police: This proposal was edited at 2025-04-23 15:58:56 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Item does not select using keyboard navigation, when first time open the search dialog on chat page. What is the root cause of that problem?When the search dialog is opened for the first time, the search input loses focus when arrow keys are pressed. As a result, pressing the Enter key does not trigger item selection. However, this issue does not occur when the dialog is opened subsequently—the search input retains focus during arrow key navigation, and Enter works as expected. This issue is occurred because of this condition. On the first render, areOptionsInitialized is false. It only becomes true after a component re-render triggered by options loading. During this initial phase, pressing arrow keys causes the search input to lose focus, leading to the unexpected behavior. On subsequent openings of the search dialog, the options have already been initialized, so areOptionsInitialized remains true from the start. This allows arrow key navigation without affecting the input focus, and Enter correctly selects the highlighted item. What changes do you think we should make in order to solve the problem?Solution 1To fix this issue we need to add
{isRecentSearchesDataLoaded && areOptionsInitialized && (
<>
<SearchInputSelectionWrapper Note: Also, we need to check other places where we used Solution 2We can add following code inside
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?UI bug so NA What alternative solutions did you explore? (Optional)Initialize options when the provider is mounted here (not ideal solution) useEffect(() => {
if (!isLoadingApp && !areOptionsInitialized.current && personalDetails && reports) {
const optionLists = createOptionList(personalDetails, reports);
setOptions({
reports: optionLists.reports,
personalDetails: optionLists.personalDetails,
});
areOptionsInitialized.current = true;
}
}, [isLoadingApp, personalDetails, reports]); |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
<SearchAutocompleteList ... textInputRef={textInputRef} />;
onLayout={() => {
...
ref.current?.updateExternalTextInputFocus?.(textInputRef.current.isFocused())
}} What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
areOptionsInitialized ? <SearchInputSelectionWrapper /> : <SearchInputSelectionWrapperSkeleton />;
|
Job added to Upwork: https://www.upwork.com/jobs/~021915316750024963357 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee ( |
@jjcoffee 👀 on the above proposals plz. |
@mallenexpensify Looks like @shubham1206agra was able to repro when the |
🚨 Edited by proposal-police: This proposal was edited at 2025-04-24 09:04:17 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.When users opens the search on chat page user can not able to open the item using keyboard navigation. What is the root cause of that problem?There are two issues. Issue 1:During first render
Because of this during first render input focus list is not initialized, so input and selectionlist focus is not sync with each other. App/src/components/Search/SearchAutocompleteInput.tsx Lines 214 to 218 in 18f248c
Issue 2:When we use tab and press enter it can not open the selected item because we haven't passed App/src/components/Search/SearchAutocompleteList.tsx Lines 600 to 601 in 8a9ecc6
What changes do you think we should make in order to solve the problem?Issue 1: To fix first issue where textbox lost focus during first load we have two approaches. Option 1 (Preffered)We can load text input only after areOptionsInitialized we can add check here or here. We don't need to add Still, if we think we should show some loader then we can add condition here and show loader for entire dialog. Options 2
App/src/components/Search/SearchRouter/SearchRouter.tsx Lines 348 to 357 in 18f248c
<SearchAutocompleteList
isInputFocused={textInputRef?.current?.isFocused()}
...
/>
Issue 1: To Fix second issue where item does not select when user use TAB we need to pass
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?None because of ui bug What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
@jjcoffee Huh... This is 4 days overdue. Who can take care of this? |
@shubham1206agra Do you want to take this (based on you being able to repro)? Happy to review the proposals if not! |
@jjcoffee I can take over this issue. |
Thanks everyone for their proposals. @thelullabyy's proposal looks good to me. 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@shubham1206agra Not sure if I understand correctly, but it seems we have two issues with the 'Enter' key here. The first one, as I mentioned, is that the 'Enter' key is disabled (the steps are the same as in the original post). The second issue occurs with other proposals as well — if we press 'Tab' to focus on another item, then pressing 'Enter' still doesn’t work Screen.Recording.2025-04-29.at.20.15.55.mov |
@thelullabyy can you please try to prioritize the PR? (not above any HIGH or CRITICAL issues though, this one is just annoying and there's an easy workaround) |
@mallenexpensify Sure, the PR will be ready today |
Can @shubham1206agra be assigned here please @strepanier03. Thanks! |
@shubham1206agra are we waiting on you to review the PR? Assigned you to this issue and removed @jjcoffee |
Issue not reproducible during KI retests. (First week) |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.44-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-05-20. 🎊 For reference, here are some details about the assignees on this issue:
|
@shubham1206agra @strepanier03 @shubham1206agra The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
Payment summary
|
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalTest:
Do we agree 👍 or 👎 |
@shubham1206agra - I've made the reg test GH and removed the hold from the payment summary, feel free to request and I'll close this out. Thanks all! |
Uh oh!
There was an error while loading. Please reload this page.
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number:
Reproducible in staging?: Needs Reproduction
Reproducible in production?: Needs Reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @mallenexpensify
Slack conversation (hyperlinked to channel name): #Expensify Bugs
Action Performed:
return
key once the contact is highlightedExpected Result:
Opens highlighted/selected chat with person
Actual Result:
Nothing happens, have to mouse to name and click on trackpad to open
Workaround:
Unknown
Platforms:
Select the officially supported platforms where the issue was reproduced:
Platforms Tested:
On which of our officially supported platforms was this issue tested:Screenshots/Videos
Add any screenshot/video evidence
2025-04-23_10-02-15.mp4
logs-2025-04-23 08_02_40.009.txt
Profile_trace_for_9.1.31-2.cpuprofile
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @strepanier03The text was updated successfully, but these errors were encountered: