Skip to content

Fix gsd mode pinned room not on the top#6849

Merged
Luke9389 merged 10 commits into
Expensify:mainfrom
K4tsuki:gsd_mode_fix
Jan 11, 2022
Merged

Fix gsd mode pinned room not on the top#6849
Luke9389 merged 10 commits into
Expensify:mainfrom
K4tsuki:gsd_mode_fix

Conversation

@K4tsuki

@K4tsuki K4tsuki commented Dec 21, 2021

Copy link
Copy Markdown
Contributor

Details

Fixed Issues

$ #6563

Tests

  1. On LHN there must be two or more pinned reports
  2. On LHN there must be one or more unpinned unread reports or have outstanding IOU
  3. The unread reports name should be between or before the pinned reports ordered alphabetically
  4. Change LHN mode to focus mode
  5. The pinned reports must be on top, alphabetically ordered
  6. The unpinned reports that have unread or outstanding IOU must be bellow pinned reports ordered alphabetically

QA Steps

  1. On LHN there should be two or more pinned reports
  2. On LHN there must be one or more unpinned unread reports or have outstanding IOU
  3. The unread report names should be between or before the pinned reports ordered alphabetically
  4. Change LHN mode to focus mode
  5. The pinned reports must be on top, ordered alphabetically
  6. The unpinned reports that have unread or outstanding IOU must be bellow pinned reports ordered alphabetically

Tested On

  • [v] Web
  • [v] Mobile Web
  • Desktop
  • iOS
  • [v] Android

Screenshots

Before

focus_before


After

Web

focus_web

Mobile Web

focus_mobile_web

Desktop

iOS

Android

focus_anroid_2

@github-actions

github-actions Bot commented Dec 21, 2021

Copy link
Copy Markdown
Contributor

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@K4tsuki

K4tsuki commented Dec 21, 2021

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@MelvinBot MelvinBot removed the request for review from a team December 22, 2021 14:54
Comment thread tests/unit/OptionsListUtilsTest.js Outdated
Comment thread tests/unit/OptionsListUtilsTest.js Outdated
Comment thread tests/unit/OptionsListUtilsTest.js Outdated
Comment thread tests/unit/OptionsListUtilsTest.js Outdated
Comment thread src/libs/OptionsListUtils.js Outdated
@parasharrajat

Copy link
Copy Markdown
Member

Also, it's strange but I can't see who signed the commits. Could you please resign your commits so that we can see your name with the GPG key?

Comment thread tests/unit/OptionsListUtilsTest.js Outdated
Comment thread tests/unit/OptionsListUtilsTest.js Outdated
@K4tsuki

K4tsuki commented Dec 23, 2021

Copy link
Copy Markdown
Contributor Author

Also, it's strange but I can't see who signed the commits. Could you please resign your commits so that we can see your name with the GPG key?

I have signed the last commit.

@parasharrajat

parasharrajat commented Dec 23, 2021

Copy link
Copy Markdown
Member

@K4tsuki You would need to sign all the commits and also please purge the last four unnecessary commits. You would need to rebase them. Thanks.

@K4tsuki

K4tsuki commented Dec 24, 2021

Copy link
Copy Markdown
Contributor Author

@K4tsuki You would need to sign all the commits and also please purge the last four unnecessary commits. You would need to rebase them. Thanks.

Done

parasharrajat
parasharrajat previously approved these changes Dec 27, 2021

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

cc: @Luke9389

@mallenexpensify

Copy link
Copy Markdown
Contributor

@Luke9389 is back on Monday and will review then, apologies for the delay @K4tsuki

@K4tsuki

K4tsuki commented Dec 29, 2021

Copy link
Copy Markdown
Contributor Author

@Luke9389 is back on Monday and will review then, apologies for the delay @K4tsuki

No problem @mallenexpensify .

Luke9389
Luke9389 previously approved these changes Jan 4, 2022

@Luke9389 Luke9389 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks!

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🎀 👀 🎀 C+ reviewed

@K4tsuki

K4tsuki commented Jan 10, 2022

Copy link
Copy Markdown
Contributor Author

@Luke9389 All cheks have passed.

@Luke9389 Luke9389 merged commit 2e5b6c3 into Expensify:main Jan 11, 2022
@OSBotify

Copy link
Copy Markdown
Contributor

✋ 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

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @Luke9389 in version: 1.1.27-2 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @Luke9389 in version: 1.1.27-3 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @Luke9389 in version: 1.1.27-3 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.29-5 🚀

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

@K4tsuki

K4tsuki commented Jan 14, 2022

Copy link
Copy Markdown
Contributor Author

@parasharrajat How about sorting issue here?
Currently the sorting is case sensitive. Should we make it case insensitive?
Should we make another pull request?

@mallenexpensify

Copy link
Copy Markdown
Contributor

@parasharrajat can you reply to @K4tsuki above?
#6849 (comment)

@parasharrajat

Copy link
Copy Markdown
Member

Aha, that's a good point. But this is a question for slack. @K4tsuki

@K4tsuki

K4tsuki commented Jan 22, 2022

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants