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

[CP Staging] Revert "Redesign search to always show search input(mobile + desktop)." #56053

Merged
merged 1 commit into from
Jan 30, 2025
Merged

[CP Staging] Revert "Redesign search to always show search input(mobile + desktop)." #56053

merged 1 commit into from
Jan 30, 2025
+673 −687

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Jan 30, 2025 •
edited
Loading

Reverts #55078

MOBILE-EXPENSIFY:13395

Fixes #56032

@mountiny mountiny self-assigned this Jan 30, 2025
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented Jan 30, 2025

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

@github-actions GitHub Actions
Copy link
Contributor

🚧 @mountiny has triggered a test hybrid app build. You can view the workflow run here.

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

🚧 @mountiny has triggered a test build. You can view the workflow run here.

NikkiWines
NikkiWines approved these changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

👍

@github-actions GitHub Actions
Copy link
Contributor

🚧 @mountiny has triggered a test hybrid app build. You can view the workflow run here.

@github-actions GitHub Actions
Copy link
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖 iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/56053/index.html https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/56053/index.html
Android iOS
Desktop 💻 Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/56053/NewExpensify.dmg https://56053.pr-testing.expensify.com
Desktop Web

👀 View the workflow run that generated this build 👀

@github-actions GitHub Actions
Copy link
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖 iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/56053/index.html https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/56053/index.html
Android iOS
Desktop 💻 Web 🕸️
N/A N/A
N/A N/A

👀 View the workflow run that generated this build 👀

@Kicu
Copy link
Member

Kicu commented Jan 30, 2025 •
edited
Loading

@mountiny I did some manual comparisong between build from this branch (with the revert) and current main using React Profiler.
Of course its only rudimentary and inconclusive, but other than 1 slightly longer react commit in the beginning (longer on main) I don't see any huge differences.

Unfortunately its also slow before @SzymczakJ PR and has been for a few weeks now 🤷 check my videos, they are maybe somewhat helpful

rec-search-main.mp4
rec-search-reverted.mp4

To sum up I see some differences but to me these are inconclusive. We will be looking into speeding Search up regardless whether we do a revert or not.

@mountiny mountiny changed the title Revert "Redesign search to always show search input(mobile + desktop)." [CP Staging] Revert "Redesign search to always show search input(mobile + desktop)." Jan 30, 2025
@mountiny mountiny marked this pull request as ready for review January 30, 2025 15:16
@mountiny mountiny requested review from a team as code owners January 30, 2025 15:16
@mountiny mountiny merged commit 649c720 into main Jan 30, 2025
16 of 21 checks passed
@mountiny mountiny deleted the revert-55078-@szymczak/add-search-input-to-mobile-search-page branch January 30, 2025 15:16
@melvin-bot melvin-bot bot added the Emergency label Jan 30, 2025
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented Jan 30, 2025

@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

@melvin-bot melvin-bot bot requested a review from techievivek January 30, 2025 15:17
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented Jan 30, 2025

@techievivek Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot removed the request for review from a team January 30, 2025 15:17
@mountiny
Copy link
Contributor Author

Straight revert, qa confirmed the delay is not present in the revert

@OSBotify
Copy link
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 pushed a commit that referenced this pull request Jan 30, 2025
@mountiny @OSBotify
Merge pull request #56053 from Expensify/revert-55078-@szymczak/add-s…
cedf153 
…earch-input-to-mobile-search-page

(cherry picked from commit 649c720)

(CP triggered by mountiny)
@github-actions github-actions bot added the CP Staging marks PRs that have been CP'd to staging label Jan 30, 2025
@github-actions github-actions bot mentioned this pull request Jan 30, 2025
Closed
48 tasks
@amyevans amyevans mentioned this pull request Jan 30, 2025
Closed
8 tasks
@github-actions GitHub Actions
Copy link
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.0.92-2 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@github-actions GitHub Actions
Copy link
Contributor

🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.92-6 🚀

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

@289Adam289 289Adam289 mentioned this pull request Jan 31, 2025
Open
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NikkiWines NikkiWines NikkiWines approved these changes

@techievivek techievivek Awaiting requested review from techievivek

Assignees

@mountiny mountiny

Labels
CP Staging marks PRs that have been CP'd to staging Ready To Build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Android & iOS - Reports - Reports page is not opened instantly
4 participants
@mountiny @Kicu @OSBotify @NikkiWines