-
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
[VIP-Travel] Create a New Travel Page and Terms and Conditions Modal (NewDot) #38469
[VIP-Travel] Create a New Travel Page and Terms and Conditions Modal (NewDot) #38469
Conversation
Travel/travel page LHN
@cdOut and @smelaa - so we had a bit of a rollercoaster discussion on the ToS, but we do still require the modal for accepting Terms, so that's great. However, our marketing team have a few copy points: cc @jamesdeanexpensify
One last question in Slack too: https://expensify.slack.com/archives/C05S5EV2JTX/p1712789189082939?thread_ts=1712595510.349369&cid=C05S5EV2JTX Update: |
Regarding the illustration... interesting, I think I was going off of all of the pages in Account Settings which use a height of 220px: Personally I would love to standardize on one size and use it everywhere if possible, including these empty states. Thoughts on that @Expensify/design? We'd need to update the empty state components in our Figma brand guidelines. |
@WojtekBoman check out the Save button on the Display name page - notice how there is a bit more space below the button? |
@shawnborton @dubielzyk-expensify UI Issues have been fixed :) Could you check if it looks ok now? |
Will fire up a test build for us, but let me know if you have updated screenshots in the original comment as well! |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
I can't update the description of this PR because I'm not the author :( |
Bummer, but feel free to just make a new comment with the updated screenshots at least! |
What's the best way to test this using the links above? I added myself to the spotnanaTravel beta but I'm not seeing the Book Travel option show up. Any ideas @twisterdotcom? |
@shawnborton you gotta wait for the beta cache to flush before your account is added. I do see it for [email protected]: |
subtitle: 'Use Expensify Travel to get the best travel offers and manage all your business expenses in one place.', | ||
features: { | ||
saveMoney: 'Save money on your bookings', | ||
alerts: 'Get real time updates and alerts', |
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.
This should be "realtime" instead of "real time". cc @jamesdeanexpensify. (Spanish can remain as it is).
() => ( | ||
<Text> | ||
{`${translate('travel.termsAndConditions.agree')}`} | ||
<TextLink href="https://www.spotnana.com/terms/">{`${translate('travel.termsAndConditions.travelTermsAndConditions')}`}</TextLink> |
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.
Should be https://use.expensify.com/travelterms
<Text style={styles.headerAnonymousFooter}>{`${translate('travel.termsAndConditions.title')}`}</Text> | ||
<Text style={styles.mt4}> | ||
{`${translate('travel.termsAndConditions.subtitle')}`} | ||
<TextLink href="https://www.spotnana.com/terms/">{`${translate('travel.termsAndConditions.termsconditions')}.`}</TextLink> |
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.
Should be https://use.expensify.com/travelterms
</Text> | ||
<Text style={styles.mt6}> | ||
{`${translate('travel.termsAndConditions.helpDocIntro')}`} | ||
<TextLink href="https://use.expensify.com/esignagreement">{`${translate('travel.termsAndConditions.helpDoc')} `}</TextLink> |
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.
@bfitzexpensify do we have a help article ready, even if it's empty?
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.
No, but I've added a note in https://github.com/Expensify/Expensify/issues/383227 to make a PR to update this once the Help Docs are live on ExpensifyHelp
On oldDot the terms go to https://use.expensify.com/travelterms, so we should use that. Looking at it now - should we link "terms & conditions" twice? Or maybe we only need to link it at the bottom with the checkbox, as we've done on oldDot: I tried web, desktop and Android and those were my only comments. |
Assigning @rushatgabhane to take over from SWM as contributor as SWM are out for a few days (@shubham1206agra you're still the reviewer here). |
Hello everyone 👋 Please review when you can - #41493 |
Details
Implementation of the main New Travel Page and the Terms and Conditions Modal for NewDot.
Fixed Issues
$ #37102
$ #37103
Tests
Permissions.ts
file set thecanUseAllBetas
function to always return true.+
sign to go into the menu and then go intoBook travel
.Book travel
button to go further into the flow.Continue
button.AcceptSpontanaTerms
has been sent as a resolution of this flow.Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.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-NATIVE.mov
Android: mWeb Chrome
ANDROID-CHROME.mov
iOS: Native
IOS-NATIVE.mp4
iOS: mWeb Safari
IOS-SAFARI.mp4
MacOS: Chrome / Safari
CHROME.mov
MacOS: Desktop
DESKTOP.mov