chore(clerk-js): Tidy up layout and logic of PlanDetails drawer #5928
Conversation
🦋 Changeset detectedLatest commit: c5f8ddb The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| useEffect(() => { | ||
| setPlanPeriod(_planPeriod); | ||
| }, [_planPeriod]); |
There was a problem hiding this comment.
Let's not sync state like that
There was a problem hiding this comment.
Need to think more about this. We want the PlanDetails to manage its own internal planPeriod state, defaulting to the planPeriod that it's passed as a prop, and updating anytime the prop updates.
|
|
||
| const portalRoot = getClosestProfileScrollBox(mode, event); | ||
|
|
||
| if (subscription && subscription.planPeriod === planPeriod && !subscription.canceledAt) { | ||
| clerk.__internal_openPlanDetails({ | ||
| plan, | ||
| planPeriod, |
There was a problem hiding this comment.
Shouldn't this information be picked up from the plan that we are already passing ?
There was a problem hiding this comment.
I think was a bug / oversight, because without this, opening the PlanDetails drawer from the SubscriptionsList would default the planPeriod to month even when the subscription being referenced had an annual planPeriod.
There was a problem hiding this comment.
I think we need to re-think some of this stuff, here a PlanDetails can be used to display both a plan that you are not have not subscribed but also a plan that you have subscribed on. For the plans that you have subscribed on, the subscription itself should tell the UI if it for a month or annual, right ? It should not be accepted as a prop.
I think the abstraction is starting to break.
Description
Update
PlanDetailsto behave similarly to the plan cards used in the PricingTable.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change