[HOLD for payment 2024-12-09] [$125] Make sure the first result in most recents is highlighted when user uses CMD+K
From the design doc.
Recent chats is the same as our existing CMD+K command, which means:
- The router can be opened and focused via the same keyboard shortcut
- When CMD+K is pressed and no search has been entered, the top-most result in Recent chats is highlighted. Hitting the return key closes the router and navigates a member to the corresponding report
cc @luacmartins
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021852421610290735954
- Upwork Job ID: 1852421610290735954
- Last Price Increase: 2024-11-01
- Automatic offers:
- nyomanjyotisa | Contributor | 104805899
Issue Owner
Current Issue Owner: @JmillsExpensify
Job added to Upwork: https://www.upwork.com/jobs/~021852421610290735954
Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)
Upwork job price has been updated to $125
Proposal
Please re-state the problem that we are trying to solve in this issue.
Make sure the first result in most recents is highlighted when user uses CMD+K
What is the root cause of that problem?
Improvement
What changes do you think we should make in order to solve the problem?
We should save the first chat index here https://github.com/Expensify/App/blob/bfce2771d22cbecceee6cf3c064fe3f0627eb5d8/src/components/Search/SearchRouter/SearchRouterList.tsx#L176
const initiallyFocusedOptionKey = styledRecentReports.at(0)?.keyForList;
we will pass it here
https://github.com/Expensify/App/blob/bfce2771d22cbecceee6cf3c064fe3f0627eb5d8/src/components/Search/SearchRouter/SearchRouterList.tsx#L240
Additionally: From the OP I can see that the list is not suppose to scrolled to the focused item on initial render but we have a code here that scroll to the focused index
https://github.com/Expensify/App/blob/bfce2771d22cbecceee6cf3c064fe3f0627eb5d8/src/components/SelectionList/BaseSelectionList.tsx#L514
we will disable this code by passing a new param shouldNotScrollToFocusedIndex
What alternative solutions did you explore? (Optional)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Make sure the first result in most recents is highlighted when user uses CMD+K
What is the root cause of that problem?
I don't know if the above proposal states the same issue or not, but for me; the issue is the lack of adding the initiallyFocusedOptionKey prop to the <SelectionList/> component.
What changes do you think we should make in order to solve the problem?
- Adding
initiallyFocusedOptionKey={recentReports && recentReports.length > 0 ? recentReports.at(0)?.keyForList : ''}prop after theonArrowFocusprop will make sure that the first chat will be focused in recent chats if there are any. https://github.com/Expensify/App/blob/bef062b4caa7f665159dc107911e708031e648c4/src/components/Search/SearchRouter/SearchRouterList.tsx#L240
What alternative solutions did you explore? (Optional)
π£ @HusseinSamy! π£ Hey, it seems we donβt have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01dd32e60eb083d581
β Contributor details stored successfully. Thank you for contributing to Expensify!
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~0139a3178534697447
β Contributor details stored successfully. Thank you for contributing to Expensify!
Proposal
Please re-state the problem that we are trying to solve in this issue.
Make sure the first result in most recents is highlighted when user uses CMD+K
What changes do you think we should make in order to solve the problem?
The lack of an initiallyFocusedOptionKey to set the default focused item within the <SelectionList> component causes the list to open without any item pre-highlighted.
To ensure the first chat in the recent chats list is automatically highlighted when the search bar opens with CMD+K, we will:
- Add a variable,
initiallyFocusedOptionKey, set to the key of the first recent chat. - Pass this variable as a prop to the
<SelectionList>component, enabling automatic highlighting.
https://github.com/Expensify/App/blob/bef062b4caa7f665159dc107911e708031e648c4/src/components/Search/SearchRouter/SearchRouterList.tsx#L176
const initiallyFocusedOptionKey = styledRecentReports.at(0)?.keyForList;
and we will pass initiallyFocusedOptionKey to <SelectionList>
https://github.com/Expensify/App/blob/bef062b4caa7f665159dc107911e708031e648c4/src/components/Search/SearchRouter/SearchRouterList.tsx#L240
initiallyFocusedOptionKey={initiallyFocusedOptionKey}
What alternative solutions did you explore? (Optional)
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~017a4e9874f15ccbd9
β Contributor details stored successfully. Thank you for contributing to Expensify!
@samranahm Your proposal will be dismissed because you did not follow the proposal template.
Edited by proposal-police: This proposal was edited at 2024-11-04 10:01:40 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Make sure the first result in most recents is highlighted when user uses CMD+K
What is the root cause of that problem?
Changes request
What changes do you think we should make in order to solve the problem?
All proposals above not working if we reload the page and directly press CMD+K, also not working if the user remove the search term. So I propose the following:
Create a new function called setInitialFocus here, and calculated the initialFocusIndex base on sortedRecentSearches and contextualReportData
const setInitialFocus = useCallback(() => {
const initialFocusIndex = sortedRecentSearches?.slice(0, 5).length + (contextualReportData ? 1 : 0);
listRef.current?.setFocusedIndex(initialFocusIndex);
listRef.current?.scrollToIndex(0, false);
}, [sortedRecentSearches, contextualReportData]);
Call it in useEffect
useEffect(() => {
setInitialFocus();
}, [sortedRecentSearches]);
And on onSearchChange, if newUserQuery false https://github.com/Expensify/App/blob/0647baf7b70967a707df6eae6cb4c3f5a260a8f5/src/components/Search/SearchRouter/SearchRouter.tsx#L307-L309
} else {
setInitialFocus();
}
POC
https://github.com/user-attachments/assets/fd61f92f-76b8-4ae0-9200-e7739b62ad83
What alternative solutions did you explore? (Optional)
@JmillsExpensify, @Pujan92, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@JmillsExpensify, @Pujan92, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@Pujan92 did you get a chance to look at the proposals above?
Yes, let's please prioritize this one. It's coming up actively with our members.
Sorry for the delay, I will review the proposals today.
Usually initiallyFocusedOptionKey is a straightforward option which also consistent with other places but it won't work for all scenarios(1. Reload and the context may not be available initially 2. When a search term is cleared) as @nyomanjyotisa mentioned. With that, I think @nyomanjyotisa's proposal of setting the focused index explicitly makes sense.
πππ C+ reviewed
Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
π£ @nyomanjyotisa π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
@JmillsExpensify, @Pujan92, @luacmartins, @nyomanjyotisa Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@JmillsExpensify, @Pujan92, @luacmartins, @nyomanjyotisa Eep! 4 days overdue now. Issues have feelings too...
PR is being worked on. Hopefully up for review this week.
β οΈ Looks like this issue was linked to a Deploy Blocker here
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.
If a regression has occurred and you are the assigned CM follow the instructions here.
If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.69-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/52298
If no regressions arise, payment will be issued on 2024-12-09. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @Pujan92 requires payment through NewDot Manual Requests
- @nyomanjyotisa requires payment automatic offer (Contributor)
@Pujan92 @JmillsExpensify @Pujan92 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]