Upgrading from Hobby to Pro displays a dialog to confirm #1545
Conversation
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/webapp/app/components/primitives/Spinner.tsxOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe changes in this pull request focus on enhancing the user interface and interaction logic within the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
668-671: Consider enhancing the upgrade messaging with more detailsThe current message about pro-rata billing could be more specific to help users make an informed decision.
Consider adding:
- Actual pro-rata calculation example
- Specific billing date
- Current vs. new plan feature comparison
- Are you sure you want to upgrade to the Pro plan? You will be charged the new - plan price for the remainder of this month on a pro rata basis. + Are you sure you want to upgrade to the Pro plan? You will be charged: + • The new plan price (${plan.tierPrice}) prorated for the remaining days this month + • Your next full billing cycle starts on [billing_date] + + You'll get immediate access to: + • [List key Pro features not in Hobby]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx(3 hunks)
🔇 Additional comments (3)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (3)
8-8: LGTM!
The new icon import is consistent with the existing icon import pattern and is appropriately used in the upgrade dialog.
636-640: LGTM!
The dialog state management follows React best practices and maintains consistency with other tier components.
653-688: LGTM!
The upgrade dialog implementation is well-structured with:
- Clear conditional rendering logic
- Consistent dialog pattern usage
- Appropriate loading states
- Informative messaging about billing implications
| : `Upgrade to ${plan.title}`} | ||
| </Button> | ||
| {subscription?.plan !== undefined && | ||
| subscription?.plan?.title === "Hobby" && |
There was a problem hiding this comment.
Instead of this, I'd do:
subscription?.plan?.type === "paid" &&
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/webapp/app/routes/storybook/route.tsx (1)
Line range hint
1-1:⚠️ Potential PR Scope MismatchWhile all the code changes related to the Spinner component are well-implemented, there appears to be a significant mismatch between:
- PR Objective: "Upgrading from Hobby to Pro displays a dialog to confirm"
- Actual Changes: Spinner component styling and Storybook documentation
Please verify if:
- These changes were committed to the wrong PR
- The PR description needs updating
- The dialog-related changes are missing from the PR
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (3)
653-688: Consider extracting the plan comparison logicThe condition for showing the upgrade dialog is complex. Consider extracting it into a helper function for better readability.
+const shouldShowUpgradeDialog = (subscription: SubscriptionResult | undefined, planCode: string) => + subscription?.plan !== undefined && + subscription?.plan?.type === "paid" && + subscription?.plan?.code !== planCode && + subscription.canceledAt === undefined; -{subscription?.plan !== undefined && -subscription?.plan?.type === "paid" && -subscription?.plan?.code !== plan.code && -subscription.canceledAt === undefined ? ( +{shouldShowUpgradeDialog(subscription, plan.code) ? (
669-672: Consider adding specific pricing informationThe dialog message could be more helpful by including the specific price difference and pro-rata calculation.
-Are you sure you want to upgrade to the Pro plan? You will be charged the new -plan price for the remainder of this month on a pro rata basis. +Are you sure you want to upgrade to the Pro plan? You will be charged ${plan.tierPrice} +for the remainder of this month (pro-rated from your current ${subscription?.plan?.price}).
690-712: Enhance button accessibility and maintainabilityConsider these improvements:
- Extract the button text logic to a helper function
- Add aria-label for better accessibility
+const getButtonText = (subscription: SubscriptionResult | undefined, planTitle: string) => { + if (!subscription?.plan) return "Select plan"; + if (subscription.plan.type === "free" || subscription.canceledAt !== undefined) + return `Upgrade to ${planTitle}`; + return subscription.plan.code === plan.code ? "Current plan" : `Upgrade to ${planTitle}`; +}; <Button variant="tertiary/large" fullWidth form="subscribe-pro" className="text-md font-medium" + aria-label={`${getButtonText(subscription, plan.title)} button`} disabled={ isLoading || (subscription?.plan?.code === plan.code && subscription.canceledAt === undefined) } LeadingIcon={ isLoading && navigation.formData?.get("planCode") === plan.code ? Spinner : undefined } > - {subscription?.plan === undefined - ? "Select plan" - : subscription.plan.type === "free" || subscription.canceledAt !== undefined - ? `Upgrade to ${plan.title}` - : subscription.plan.code === plan.code - ? "Current plan" - : `Upgrade to ${plan.title}`} + {getButtonText(subscription, plan.title)} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/webapp/app/components/primitives/Spinner.tsx(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx(3 hunks)apps/webapp/app/routes/storybook.spinner/route.tsx(1 hunks)apps/webapp/app/routes/storybook/route.tsx(1 hunks)
🔇 Additional comments (5)
apps/webapp/app/routes/storybook.spinner/route.tsx (1)
1-28: LGTM! Well-structured Storybook story for Spinner component.
The implementation provides clear examples of all Spinner variants with appropriate styling and backgrounds. However, this change appears unrelated to the PR's objective of adding a confirmation dialog for Hobby to Pro tier upgrades.
Let's verify if these files are intended for this PR:
apps/webapp/app/components/primitives/Spinner.tsx (1)
29-30: LGTM! Color adjustments improve visibility.
The updated RGBA values for the dark variant provide better contrast while maintaining the component's aesthetic. However, like the previous file, this change appears unrelated to the PR's objective.
apps/webapp/app/routes/storybook/route.tsx (1)
87-90: LGTM! New Storybook entry follows established pattern.
The addition of the Spinners story is well-structured. However, I have concerns about the scope of this PR:
- The PR description indicates adding a confirmation dialog for Hobby to Pro tier upgrades
- The actual changes are related to Spinner component styling and documentation
- This suggests these changes might belong to a different PR
Let's verify the PR description and files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2)
636-640: LGTM: Clean state management implementation
The dialog state management follows React best practices with proper state reset on subscription changes.
653-713: Verify the upgrade flow matches the requirements
The implementation successfully adds the confirmation dialog for upgrading from Hobby to Pro plan. Please verify:
- Dialog only appears when upgrading from Hobby to Pro
- Pro-rata pricing message is accurate
- Dialog doesn't appear for Free to Pro upgrades
✅ Verification successful
Implementation correctly handles plan upgrade scenarios
The verification confirms the implementation meets the requirements:
- The upgrade dialog appears only when:
- Current plan is paid (Hobby)
- Target plan code is different
- Subscription is not canceled
- Pro-rata pricing message is unique to this dialog and consistent
- Dialog correctly doesn't appear for Free to Pro upgrades (handled in the else branch with direct button)
- All edge cases are properly handled:
- Free plan upgrades
- Canceled subscriptions
- Current plan selection
- Loading states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the upgrade dialog implementation is consistent across the codebase
# Check if there are any other upgrade dialogs that might conflict
rg -A 5 "Upgrade plan\?" --type tsx
# Check for any other pro-rata pricing messages to ensure consistency
rg -A 5 "pro rata basis" --type tsx
Length of output: 131
Script:
#!/bin/bash
# Description: Verify the upgrade dialog implementation is consistent across the codebase
# Check if there are any other upgrade dialogs that might conflict
rg -A 5 "Upgrade plan\?"
# Check for any other pro-rata pricing messages to ensure consistency
rg -A 5 "pro rata basis"
# Check for other plan upgrade related code
rg -A 5 "Upgrade to.*plan"
Length of output: 5536
Script:
#!/bin/bash
# Let's verify the conditional logic for showing the dialog
# Check the conditions for when dialog appears
ast-grep --pattern 'subscription?.plan?.type === "paid"'
# Check for free plan type conditions
ast-grep --pattern 'subscription.plan.type === "free"'
# Look for canceledAt checks
ast-grep --pattern 'subscription.canceledAt'
Length of output: 3952
If you click on the Upgrade to Pro button while on the Hobby tier, you will see this Dialog
This dialog doesn't appear if you click on "Upgrade to Pro" if you're on the Free tier.
Downgrading from any tier still functions the same.
Summary by CodeRabbit
New Features
Storycomponent to showcase various spinner configurations.Bug Fixes
Style
Spinnercomponent with new RGBA color configurations.Chores