Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix: search page order #6544

Merged
merged 3 commits into from
Dec 1, 2021
Merged

Fix: search page order #6544

merged 3 commits into from
Dec 1, 2021

Conversation

parasharrajat
Copy link
Member

Details

Fixed Issues

$ #6359

Tests | QA Steps

  1. Open the Search page.
  2. Search for something.
  3. Order of results should be [HOLD for payment 2021-12-15] Search - Make users appear first in the result list over group chats or rooms #6359 (comment).

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web | Android | Mobile Web | Desktop | iOS

output_file.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner November 30, 2021 19:16
@MelvinBot MelvinBot requested review from Jag96 and removed request for a team November 30, 2021 19:16
Jag96
Jag96 reviewed Dec 1, 2021
View reviewed changes
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed is that the #announce rooms are still showing above some group chats:
image image

For example in those the #announce room should be below the 1:1s and the group chats

@parasharrajat
Set ordering parameter for Search page.
62d8e63 
Order is as below:
1. 1:1 chat.
2. group chat.
3. rooms.
@parasharrajat
Copy link
Member Author

parasharrajat commented Dec 1, 2021 •
edited
Loading

@Jag96 I have fixed it. Actually, we have to change the Initial proposal.
Now I am ordering the recent reports in the requested order based on the newly added parameter.
There is a known bug due to which room names are duplicated on each search. The good news is I have found the fix for that as well. I will propose it on the related issue #5699.

@parasharrajat
Copy link
Member Author

@Jag96 Also, could you please get #5699 rolling? No one is assigned to that issue.

Jag96
Jag96 suggested changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nitpick, otherwise, this looks good and works well!

src/libs/OptionsListUtils.js Outdated Show resolved Hide resolved
@Jag96 Jag96 merged commit d94a8b6 into Expensify:main Dec 1, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Dec 1, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify OSBotify mentioned this pull request Dec 6, 2021
Closed
95 tasks
@OSBotify
Copy link
Contributor

OSBotify commented Dec 7, 2021

🚀 Deployed to staging by @Jag96 in version: 1.1.17-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@kavimuru kavimuru mentioned this pull request Dec 7, 2021
Closed
@botify botify mentioned this pull request Dec 8, 2021
Closed
@OSBotify
Copy link
Contributor

OSBotify commented Dec 8, 2021

🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@parasharrajat parasharrajat deleted the searchpage branch November 20, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jag96 Jag96 Jag96 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@parasharrajat @OSBotify @Jag96