-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
[No QA] Update force upgrade modal #52703
[No QA] Update force upgrade modal #52703
Conversation
cc @staszekscp |
I checked the changes locally and it worked but as @Julesssss said we're not able to test it reliably without code changes |
This doesn't show the upgrade modal on Web or Desktop, right? |
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.
Hey @war-in, @AndrewGable createdand deployed a BETA that we can use here to safety test on production: hybridForcedMigration
.
Actually sorry you can ignore that. I'll be handling on the backend entirely 😄
@AndrewGable there are no changes for web or desktop so it'll work like previously |
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.
@war-in can this be put into full review?
@Julesssss done! |
🚧 @Julesssss has triggered a test build. You can view the workflow run here. |
Does this need a C+ review? |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Hi @c3024, I don't think so in this case as we're keeping a close eye on it internally. |
@AndrewGable I think we're good here |
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.
Coming from the thread, copy edit for this one.
…g-prod-env-upgrade-modal
@trjExpensify done |
Lets test against the beta. Looks good to me 👍 |
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.
Modal looks good to me!
FYI conflicts |
Co-authored-by: Tom Rhys Jones <[email protected]>
…upgrade-modal # Conflicts: # src/languages/es.ts
Conflicts resolved! |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.0.66-0 🚀
|
Checked off App deploy internalQA, we need some more PRs to be deployed first. |
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.66-8 🚀
|
Explanation of Change
Fixed Issues
$ #52276
PROPOSAL:
Tests
We're not able to test this change without mocking some local variables
Offline tests
QA Steps
Internal testing only!
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop