-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[CP Staging] [Revert] Perf/onyx upgrade use cache directly #24003
Conversation
@aimane-chnaif 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] |
Oops, sad news to revert this feature 😞 |
I am working full speed on getting it back merged! @aimane-chnaif 😊 |
Tested, LGTM! |
Thanks @hannojg ! Not sure why i can't request you as a reviewer 😢 I'll look for a quick reviewer so we can get this CP'd! |
Testing right now |
Aah ok thanks @aimane-chnaif I will wait for you 🙏 |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-08-01.at.3.44.54.PM.movMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that 4 linked issues are not happening anymore
@aldo-expensify 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] |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…Access [CP Staging] [Revert] Perf/onyx upgrade use cache directly (cherry picked from commit a3c2e1b)
This was also a regression of the PR: |
🚀 Deployed to staging by https://github.com/cristipaval in version: 1.3.48-3 🚀
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.3.48-5 🚀
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 1.3.50-0 🚀
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.3.50-3 🚀
|
cc @hannojg 🙏
Details
Reverting #23749 b/c it has caused a few unexpected issues, multiple deploy blockers
Fixed Issues
$ #23950
$ #23962
$ #23951
$ #23973
$ #24006
Tests
Offline tests
Same as above
QA Steps
Same as above
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android