-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[CP Staging] fix: trip booking not navigate to T&C Page #49877
Conversation
I will review this PR because this is regression from my previous PR |
@daledah Prettier check failed |
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.
LGTM
@daledah lint is failing |
@eh2077 are you available to review this PR? It fixes a blocker so we wanna get it merged soon |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-09-30.at.23.28.54.movAndroid: mWeb ChromeScreen.Recording.2024-09-30.at.23.32.39.moviOS: NativeScreen.Recording.2024-09-30.at.23.27.13.moviOS: mWeb SafariScreen.Recording.2024-09-30.at.23.31.24.movMacOS: Chrome / SafariScreen.Recording.2024-09-30.at.23.23.51.movMacOS: DesktopScreen.Recording.2024-09-30.at.23.26.05.mov |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
[CP Staging] fix: trip booking not navigate to T&C Page (cherry picked from commit f915029) (CP triggered by jasperhuangg)
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.0.41-7 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
Confirmed the fix on staging |
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.0.41-10 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.41-10 🚀
|
Details
Fixed Issues
$ #49870
PROPOSAL:
Tests
6 Verify that: T&C Page is shown
Offline tests
QA Steps
6 Verify that: T&C Page is shown
PR Author Checklist
### Fixed Issues
section above- [x] I added steps for local testing in the
Tests
section- [x] I added steps for the expected offline behavior in the
Offline steps
section- [x] I added steps for Staging and/or Production testing in the
QA steps
section- [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
- [x] Android: Native
- [x] Android: mWeb Chrome
- [x] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
- [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e.
toggleReport
and notonIconClick
)- [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g.
myBool && <MyComponent />
.- [x] I verified that comments were added to code that is not self explanatory
- [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- [x] I verified any copy / text shown in the product is localized by adding it to
src/languages/*
files and using the translation method- [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- [x] I verified the JSDocs style guidelines (in
STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)- [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
- [x] A similar style doesn't already exist
- [x] The style can't be created with an existing StyleUtils function (i.e.
StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)- [x] I verified that all the inputs inside a form are aligned with each other.
- [x] I added
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
Screen.Recording.2024-09-28.at.13.28.48.mp4
Android: mWeb Chrome
Screen.Recording.2024-09-28.at.13.23.08.mov
iOS: Native
Screen.Recording.2024-09-28.at.13.34.53.mov
iOS: mWeb Safari
Screen.Recording.2024-09-28.at.13.39.11.mov
MacOS: Chrome / Safari
Screen.Recording.2024-09-28.at.13.22.20.mov
MacOS: Desktop
Screen.Recording.2024-09-28.at.13.41.58.mov