-
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
[Free trial] Implement and show Free Trial banner and badges in the App after Free Trial starts #44371
Conversation
…ow-free-trial-banner-and-badges-after-free-trial-starts # Conflicts: # src/pages/settings/Subscription/CardSection/CardSection.tsx
…ow-free-trial-banner-and-badges-after-free-trial-starts
src/pages/settings/Subscription/CardSection/BillingBanner/TrialBillingBanner.tsx
Outdated
Show resolved
Hide resolved
src/pages/settings/Subscription/CardSection/BillingBanner/TrialBillingBanner.tsx
Outdated
Show resolved
Hide resolved
…ow-free-trial-banner-and-badges-after-free-trial-starts
@hoangzinh 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] |
src/pages/settings/Subscription/CardSection/BillingBanner/TrialStartedBillingBanner.tsx
Outdated
Show resolved
Hide resolved
…ow-free-trial-banner-and-badges-after-free-trial-starts # Conflicts: # src/pages/settings/Subscription/CardSection/CardSection.tsx
…ow-free-trial-banner-and-badges-after-free-trial-starts
…ow-free-trial-banner-and-badges-after-free-trial-starts
@hoangzinh I fixed the border color issue! But the issue with the text getting cropped is expected, because this is caused by the right icon that is shown currently, since the subscription button redirects the user to the old dot subscription page. |
@dannymcclain can you give this a looksie today as well, as you lead the design on it? Thanks! |
…ow-free-trial-banner-and-badges-after-free-trial-starts
@hoangzinh feedback applied 😄 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-06-28.at.21.26.15.android.movAndroid: mWeb ChromeScreen.Recording.2024-06-28.at.21.44.21.android.chrome.moviOS: NativeScreen.Recording.2024-06-28.at.21.40.20.ios.moviOS: mWeb SafariScreen.Recording.2024-06-28.at.21.43.10.ios.safari.movMacOS: Chrome / SafariScreen.Recording.2024-06-28.at.20.49.42.web.mp4MacOS: DesktopScreen.Recording.2024-06-28.at.20.54.13.desktop.mov |
This is looking good! Small suggestion: can we remove the word "below" from the message about adding a payment card? That way it will make more sense when viewing in native apps (since there's no option to add a payment card). CurrentFree trial: X days left! ProposalFree trial: X days left! The banner is so close to the big add payment card button that I don't really think we need to say "below" in that message. |
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.
Code LGTM, not merging since danny has a change requested here.
Let me know once addressed.
…ow-free-trial-banner-and-badges-after-free-trial-starts
@dannymcclain I just pushed the change you requested 😄 @chiragsalian I think we're good to go now! |
@pac-guerreiro It appears that we just updated the text in English, how about the text in Spanish? |
@hoangzinh the spanish translation doesn't included the translation for the word |
Looks like you've got some merge conflicts. Can you address them. |
…ow-free-trial-banner-and-badges-after-free-trial-starts # Conflicts: # src/pages/settings/Subscription/CardSection/CardSection.tsx
@chiragsalian 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/chiragsalian in version: 9.0.4-0 🚀
|
🚀 Cherry-picked to staging by https://github.com/tgolen in version: 9.0.4-5 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.6-8 🚀
|
Details
Fixed Issues
$ #43672
PROPOSAL: N/A
Tests
/settings/subscription
and verify that the trial duration banner is shown in the card sectionOffline 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 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