App icon indicating copy to clipboard operation
App copied to clipboard

[HOLD for payment 2024-12-17] Make Search Input on search results Page behave similarly to SearchRouter

Open Kicu opened this issue 1 year ago • 14 comments

Followup after discussion in: https://github.com/Expensify/App/pull/52568

Make the Input on SearchResults Page behave similarly to SearchRouter. It should show:

  • 5 recent searches same as Router
  • chat rooms/reports working identically to router (navigating to chats when clicked)

Also make cmd+k on this Page focus the input instead of displaying Router.

Screenshot 2024-11-26 at 13 13 06

CC @shawnborton @luacmartins

Issue OwnerCurrent Issue Owner: @trjExpensify

Kicu avatar Nov 26 '24 12:11 Kicu

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] avatar Nov 26 '24 12:11 melvin-bot[bot]

Awesome, thanks!

shawnborton avatar Nov 26 '24 12:11 shawnborton

Nice, who's taking this one on @Kicu?

trjExpensify avatar Nov 26 '24 13:11 trjExpensify

myself ofcourse! 😁 please assign me 🙏

Kicu avatar Nov 26 '24 13:11 Kicu

Done!

luacmartins avatar Nov 26 '24 18:11 luacmartins

@Kicu, @trjExpensify, @luacmartins Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] avatar Dec 02 '24 09:12 melvin-bot[bot]

PR will be ready today. Recently a lot of people are touching Search simultaneously, and I had a lot of problems resolving conflicts on Friday.

Kicu avatar Dec 02 '24 09:12 Kicu

Please also assign me to the issue here. Thanks.

rojiphil avatar Dec 05 '24 17:12 rojiphil

PR merged

luacmartins avatar Dec 05 '24 17:12 luacmartins

⚠️ 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.

melvin-bot[bot] avatar Dec 09 '24 20:12 melvin-bot[bot]

PR merged

luacmartins avatar Dec 10 '24 19:12 luacmartins

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.73-8 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/53198

If no regressions arise, payment will be issued on 2024-12-17. :confetti_ball:

For reference, here are some details about the assignees on this issue:

  • @rojiphil requires payment (Needs manual offer from BZ)
  • @Kicu does not require payment (Contractor)

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

@rojiphil @trjExpensify @rojiphil 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]

melvin-bot[bot] avatar Dec 10 '24 22:12 melvin-bot[bot]

This had a regression:

  • https://github.com/Expensify/App/issues/53823

which was confirmed by author in follow-up PR https://github.com/Expensify/App/pull/53859 description.

ikevin127 avatar Dec 16 '24 18:12 ikevin127

Payment Summary

Upwork Job

  • ROLE: @rojiphil paid $(AMOUNT) via Upwork (LINK)
  • Contributor: @Kicu is from an agency-contributor and not due payment

BugZero Checklist (@trjExpensify)

  • [ ] I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • [ ] I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • [ ] I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • [ ] I have verified the payment summary above is correct

melvin-bot[bot] avatar Dec 17 '24 09:12 melvin-bot[bot]

Checklist time please, @rojiphil!

trjExpensify avatar Dec 17 '24 12:12 trjExpensify

BugZero Checklist:

  • [x] [Contributor] Classify the bug:
Bug classification

Source of bug:

  • [x] 1a. Result of the original design (eg. a case wasn't considered)
  • [ ] 1b. Mistake during implementation
  • [ ] 1c. Backend bug
  • [ ] 1z. Other:

Where bug was reported:

  • [ ] 2a. Reported on production
  • [ ] 2b. Reported on staging (deploy blocker)
  • [x] 2c. Reported on a PR
  • [ ] 2z. Other:

Who reported the bug:

  • [ ] 3a. Expensify user
  • [x] 3b. Expensify employee
  • [ ] 3c. Contributor
  • [ ] 3d. QA
  • [ ] 3z. Other:
  • [x] [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    Link to comment: This was a continual work from this PR

  • [x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

    Link to discussion: Not Required

  • [x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

Precondition:

Test:

  1. Ensure that the central pane is displaying any page other than the search page
  2. On key-in Cmd+k, verify that the search router is displayed
  3. Enter a search text e.g. type:chat from:[email protected] and press enter to open the search page.
  4. Verify that the search page shows the search input containing the search text.
  5. On key-in Cmd+k verify that the focus is shown on the search input.
  6. Key-in Cmd+k again and verify that the focus is lost on the search input.
  7. Key-in Cmd+k again and bring back the focus on the search input.
  8. Key-in a search text and press enter so that reports are also displayed in search results.
  9. Click/Tap a report/workspace in the search result and verify that the central pane is navigated to the selected report/workspace.
  10. Navigate back to the Search page and key-in the email of a user with whom there was no chat before.
  11. Press Enter and verify that the user is navigated to the chat report of the new user.

Do we agree 👍 or 👎

rojiphil avatar Dec 18 '24 02:12 rojiphil

@luacmartins Can we keep the compensation here to $250 after considering the regression? The reasoning is that we did some awesome work here and it took more cycles of review than the usual ones to reach there. cc @trjExpensify

rojiphil avatar Dec 18 '24 02:12 rojiphil

Yea, I think that's fine

luacmartins avatar Dec 18 '24 16:12 luacmartins

Sounds good!

Payment summary as follows:

  • $250 to @rojiphil for the C+ review

Offer sent!

trjExpensify avatar Dec 19 '24 11:12 trjExpensify

Accepted offer. Thanks @trjExpensify

rojiphil avatar Dec 19 '24 11:12 rojiphil

Paid, closing!

trjExpensify avatar Dec 19 '24 11:12 trjExpensify