-
Notifications
You must be signed in to change notification settings - Fork 374
fix(clerk-js): Display alert on plan details error #6384
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
Changes from all commits
01eb35a
9edfad5
4f648b2
12a55c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@clerk/clerk-js': patch | ||
--- | ||
|
||
Display alert on plan details error |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,32 @@ | ||
import { useClerk } from '@clerk/shared/react'; | ||
import type { __internal_PlanDetailsProps, CommercePlanResource, CommerceSubscriptionPlanPeriod } from '@clerk/types'; | ||
import type { | ||
__internal_PlanDetailsProps, | ||
ClerkAPIResponseError, | ||
CommercePlanResource, | ||
CommerceSubscriptionPlanPeriod, | ||
} from '@clerk/types'; | ||
import * as React from 'react'; | ||
import { useMemo, useState } from 'react'; | ||
import useSWR from 'swr'; | ||
|
||
import { Alert } from '@/ui/elements/Alert'; | ||
import { Avatar } from '@/ui/elements/Avatar'; | ||
import { Drawer } from '@/ui/elements/Drawer'; | ||
import { Switch } from '@/ui/elements/Switch'; | ||
|
||
import { SubscriberTypeContext } from '../../contexts'; | ||
import { Box, Col, descriptors, Flex, Heading, localizationKeys, Span, Spinner, Text } from '../../customizables'; | ||
import { | ||
Box, | ||
Col, | ||
descriptors, | ||
Flex, | ||
Heading, | ||
localizationKeys, | ||
Span, | ||
Spinner, | ||
Text, | ||
useLocalizations, | ||
} from '../../customizables'; | ||
|
||
export const PlanDetails = (props: __internal_PlanDetailsProps) => { | ||
return ( | ||
|
@@ -19,6 +36,39 @@ export const PlanDetails = (props: __internal_PlanDetailsProps) => { | |
); | ||
}; | ||
|
||
const BodyFiller = ({ children }: { children: React.ReactNode }) => { | ||
return ( | ||
<Drawer.Body | ||
sx={t => ({ | ||
display: 'flex', | ||
flexDirection: 'column', | ||
alignItems: 'center', | ||
justifyContent: 'center', | ||
flex: 1, | ||
overflowY: 'auto', | ||
padding: t.space.$4, | ||
gap: t.space.$4, | ||
})} | ||
> | ||
{children} | ||
</Drawer.Body> | ||
); | ||
}; | ||
|
||
const PlanDetailsError = ({ error }: { error: ClerkAPIResponseError }) => { | ||
const { translateError } = useLocalizations(); | ||
return ( | ||
<BodyFiller> | ||
<Alert | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be full width? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to force it ? If the error message is large enough it will occupy the necessary space. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
variant='danger' | ||
colorScheme='danger' | ||
> | ||
{translateError(error.errors[0])} | ||
</Alert> | ||
</BodyFiller> | ||
); | ||
}; | ||
|
||
const PlanDetailsInternal = ({ | ||
planId, | ||
plan: initialPlan, | ||
|
@@ -27,29 +77,34 @@ const PlanDetailsInternal = ({ | |
const clerk = useClerk(); | ||
const [planPeriod, setPlanPeriod] = useState<CommerceSubscriptionPlanPeriod>(initialPlanPeriod); | ||
|
||
const { data: plan, isLoading } = useSWR( | ||
const { | ||
data: plan, | ||
isLoading, | ||
error, | ||
} = useSWR<CommercePlanResource, ClerkAPIResponseError>( | ||
planId || initialPlan ? { type: 'plan', id: planId || initialPlan?.id } : null, | ||
// @ts-expect-error we are handling it above | ||
() => clerk.billing.getPlan({ id: planId || initialPlan?.id }), | ||
{ | ||
fallbackData: initialPlan, | ||
revalidateOnFocus: false, | ||
shouldRetryOnError: false, | ||
keepPreviousData: true, | ||
}, | ||
); | ||
|
||
if (isLoading && !initialPlan) { | ||
return ( | ||
<Flex | ||
justify='center' | ||
align='center' | ||
sx={{ | ||
height: '100%', | ||
}} | ||
> | ||
<BodyFiller> | ||
<Spinner /> | ||
</Flex> | ||
</BodyFiller> | ||
); | ||
} | ||
|
||
if (!plan && error) { | ||
return <PlanDetailsError error={error} />; | ||
} | ||
|
||
if (!plan) { | ||
return null; | ||
} | ||
|
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.
body already handles a majority of these.
javascript/packages/clerk-js/src/ui/elements/Drawer.tsx
Lines 351 to 355 in 79b4f03
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.
Those are not inherited tho. If i removed the duplicate ones then this is the UI I end up with.
