Skip to content
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.

Sign up for GitHub

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

feat(elements): Loading states #2669

Closed
wants to merge 5 commits into from
Closed

feat(elements): Loading states #2669

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74a9976
feat(elements): Move SignIn components to /sign-in export, remove har…
BRKalow Jan 24, 2024
1cd5919
chore(elements): Remove ClerkLoaded wrapper to allow SSRing of the in…
BRKalow Jan 25, 2024
070b52e
feat(elements): Introduce loading states
BRKalow Jan 26, 2024
f7f276f
Update packages/elements/src/internals/machines/sign-in/sign-in.machi…
BRKalow Jan 26, 2024
2028534
Merge branch 'brk.chore/refactor-elements-build' into brk.feat/loadin…
BRKalow Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions packages/elements/examples/nextjs/app/sign-in/[[...sign-in]]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import { GlobalError, Submit } from '@clerk/elements';
import {
SignIn,
Root as SignIn,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align exports with our current docs

SignInContinue,
SignInFactorOne,
SignInFactorTwo,
SignInLoading,
SignInSocialProvider,
SignInStart,
SignInStrategy,
Expand All @@ -23,9 +24,7 @@ export default function SignInPage() {
<SignInStart>
<div className='flex flex-col items-center justify-center gap-12'>
<H1>START</H1>

<GlobalError className='block text-red-400 font-mono' />

<div className='flex flex-col items-stretch justify-center gap-2'>
<SignInSocialProvider
name='github'
Expand All @@ -51,23 +50,16 @@ export default function SignInPage() {
Sign In with Metamask
</SignInSocialProvider>
</div>

<Hr />

<div className='flex gap-6 flex-col'>
<CustomField
label='Email'
name='identifier'
/>

{/* <Hr />

<CustomField
label='Phone'
name='identifier'
/> */}

<CustomSubmit>Sign In</CustomSubmit>
<CustomSubmit>
<SignInLoading fallback={<>Submitting...</>}>Sign In</SignInLoading>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example usage for rendering based on the global loading state. For further focusing we could do something like:

<SignInLoading context="oauth_github">

Ideally we can make this a non-flow-specific component, though it's hard if we rely on state node tags.

</CustomSubmit>
</div>
</div>
</SignInStart>
Expand Down
10 changes: 10 additions & 0 deletions packages/elements/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@
"types": "./dist/sign-in.d.ts",
"default": "./dist/sign-in.js"
}
},
"./server": {
"import": {
"types": "./dist/server.d.mts",
"default": "./dist/server.mjs"
},
"require": {
"types": "./dist/server.d.ts",
"default": "./dist/server.js"
}
}
},
"files": [
Expand Down
16 changes: 12 additions & 4 deletions packages/elements/src/internals/machines/sign-in/sign-in.context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ export const useSignInThirdPartyProviders = () => {

export const useSignInThirdPartyProvider = (provider: OAuthProvider | Web3Provider): UseThirdPartyProviderReturn => {
const ref = useSignInFlow();
const state = useSignInStateMatcher();
const details = useSignInFlowSelector(clerkThirdPartyProviderSelector(provider));
const strategy = provider === 'metamask' ? ('web3_metamask_signature' as const) : (`oauth_${provider}` as const);

const authenticate = useCallback(
(event: React.MouseEvent<Element>) => {
Expand All @@ -153,12 +155,16 @@ export const useSignInThirdPartyProvider = (provider: OAuthProvider | Web3Provid
event.preventDefault();

if (provider === 'metamask') {
return ref.send({ type: 'AUTHENTICATE.WEB3', strategy: 'web3_metamask_signature' });
ref.send({ type: 'SET_LOADING_CONTEXT', name: strategy });
// @ts-expect-error -- TS is not respecting the ternary in the strategy declaration above
return ref.send({ type: 'AUTHENTICATE.WEB3', strategy });
}

return ref.send({ type: 'AUTHENTICATE.OAUTH', strategy: `oauth_${provider}` });
ref.send({ type: 'SET_LOADING_CONTEXT', name: strategy });
// @ts-expect-error -- TS is not respecting the ternary in the strategy declaration above
return ref.send({ type: 'AUTHENTICATE.OAUTH', strategy });
},
[provider, details, ref],
[provider, details, ref, strategy],
);

if (!details) {
Expand All @@ -170,7 +176,9 @@ export const useSignInThirdPartyProvider = (provider: OAuthProvider | Web3Provid
events: {
authenticate,
},
...details,
isDisabled: state.hasTag('loading'),
isLoading: state.hasTag('loading') && state.context.loadingContext === strategy,
provider: details,
};
};

Expand Down
28 changes: 25 additions & 3 deletions packages/elements/src/internals/machines/sign-in/sign-in.machine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ export type SignInMachineEvents =
| { type: 'AUTHENTICATE.WEB3'; strategy: Web3Strategy }
| { type: 'FAILURE'; error: Error }
| { type: 'OAUTH.CALLBACK' }
| { type: 'SUBMIT' };
| { type: 'SUBMIT' }
| { type: 'SET_LOADING_CONTEXT'; name: string };

export interface SignInMachineTypes {
context: SignInMachineContext;
Expand Down Expand Up @@ -141,6 +142,14 @@ export const SignInMachine = setup({
};
},
),
setLoadingContext: assign(({ event }) => {
if (event.type === 'SET_LOADING_CONTEXT') {
return {
loadingContext: event.name,
};
}
return {};
}),
Comment on lines +145 to +152
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's likely a more elegant way we can store an identifier for whatever action / element triggered the loading state, this was my first stab at it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels best handled when AUTHENTICATE.OAUTH/AUTHENTICATE.[...] are called, rather than their own events, unless I'm missing something.

},
guards: {
isCurrentFactorPassword: ({ context }) => context.currentFactor?.strategy === 'password',
Expand Down Expand Up @@ -188,6 +197,7 @@ export const SignInMachine = setup({
on: {
'CLERKJS.NAVIGATE.*': '.HavingTrouble',
FAILURE: '.HavingTrouble',
SET_LOADING_CONTEXT: { actions: ['setLoadingContext'] },
},
states: {
Init: {
Expand Down Expand Up @@ -263,6 +273,11 @@ export const SignInMachine = setup({
guard: 'isSignInComplete',
target: 'Complete',
},
{
guard: 'needsIdentifier',
target: 'Start',
reenter: true,
},
{
guard: 'needsFirstFactor',
target: 'FirstFactor',
Expand All @@ -283,6 +298,7 @@ export const SignInMachine = setup({
},
},
Attempting: {
tags: ['loading'],
invoke: {
id: 'createSignIn',
src: 'createSignIn',
Expand All @@ -307,6 +323,7 @@ export const SignInMachine = setup({
},
FirstFactor: {
initial: 'DeterminingState',
// entry: ['assignStartingFirstFactor'],
BRKalow marked this conversation as resolved.
Show resolved Hide resolved
entry: [{ type: 'navigateTo', params: { path: '/continue' } }, 'assignStartingFirstFactor'],
onDone: [
{
Expand All @@ -323,7 +340,7 @@ export const SignInMachine = setup({
always: [
{
description: 'If the current factor is not password, prepare the factor',
guard: not('isCurrentFactorPassword'),
guard: and([not('isCurrentFactorPassword'), { type: 'isCurrentPath', params: { path: '/continue' } }]),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensures prepare only fires once we're on the right path

target: 'Preparing',
},
{
Expand All @@ -333,6 +350,7 @@ export const SignInMachine = setup({
],
},
Preparing: {
tags: ['loading'],
invoke: {
id: 'prepareFirstFactor',
src: 'prepareFirstFactor',
Expand Down Expand Up @@ -367,6 +385,7 @@ export const SignInMachine = setup({
},
},
Attempting: {
tags: ['loading'],
invoke: {
id: 'attemptFirstFactor',
src: 'attemptFirstFactor',
Expand Down Expand Up @@ -406,7 +425,7 @@ export const SignInMachine = setup({
always: [
{
description: 'If the current factor is not TOTP, prepare the factor',
guard: not('isCurrentFactorTOTP'),
guard: and([not('isCurrentFactorTOTP'), { type: 'isCurrentPath', params: { path: '/continue' } }]),
target: 'Preparing',
reenter: true,
},
Expand All @@ -418,6 +437,7 @@ export const SignInMachine = setup({
],
},
Preparing: {
tags: ['loading'],
invoke: {
id: 'prepareSecondFactor',
src: 'prepareSecondFactor',
Expand Down Expand Up @@ -445,6 +465,7 @@ export const SignInMachine = setup({
},
},
Attempting: {
tags: ['loading'],
invoke: {
id: 'attemptSecondFactor',
src: 'attemptSecondFactor',
Expand Down Expand Up @@ -475,6 +496,7 @@ export const SignInMachine = setup({
},
},
AuthenticatingWithRedirect: {
tags: ['loading'],
invoke: {
id: 'authenticateWithSignInRedirect',
src: 'authenticateWithSignInRedirect',
Expand Down
20 changes: 11 additions & 9 deletions packages/elements/src/react/common/form/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
Label as RadixLabel,
Submit,
} from '@radix-ui/react-form';
import type { ComponentProps, ReactNode } from 'react';
import React, { createContext, useCallback, useContext, useEffect } from 'react';
import type { ChangeEvent, ComponentProps, FormEvent, ReactNode } from 'react';
import { createContext, useCallback, useContext, useEffect, useTransition } from 'react';
import type { BaseActorRef } from 'xstate';

import type { ClerkElementsError } from '~/internals/errors/error';
Expand All @@ -40,22 +40,24 @@ const useFieldContext = () => useContext(FieldContext);
*/
const useForm = ({ flowActor }: { flowActor?: BaseActorRef<{ type: 'SUBMIT' }> }) => {
const error = useFormSelector(globalErrorsSelector);
const [isPending, startTransition] = useTransition();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing using React's native transitions. Not sure if it provides value beyond the loading state tracking within the state machine.

const validity = error ? 'invalid' : 'valid';

// Register the onSubmit handler for form submission
// TODO: merge user-provided submit handler
const onSubmit = useCallback(
(event: React.FormEvent<Element>) => {
(event: FormEvent<Element>) => {
event.preventDefault();
if (flowActor) {
flowActor.send({ type: 'SUBMIT' });
startTransition(() => flowActor.send({ type: 'SUBMIT' }));
}
},
[flowActor],
);

return {
props: {
[`data-submitting`]: isPending,
[`data-${validity}`]: true,
onSubmit,
},
Expand Down Expand Up @@ -130,7 +132,7 @@ const useInput = ({ name: inputName, value: initialValue, type: inputType, ...pa

// Register the onChange handler for field updates to persist to the machine context
const onChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
(event: ChangeEvent<HTMLInputElement>) => {
onChangeProp?.(event);
if (!name) return;
ref.send({ type: 'FIELD.UPDATE', field: { name, value: event.target.value } });
Expand All @@ -156,7 +158,7 @@ const useInput = ({ name: inputName, value: initialValue, type: inputType, ...pa
inputMode: 'numeric',
pattern: '[0-9]*',
maxLength: 6,
onChange: (event: React.ChangeEvent<HTMLInputElement>) => {
onChange: (event: ChangeEvent<HTMLInputElement>) => {
// Only accept numbers
event.currentTarget.value = event.currentTarget.value.replace(/\D+/g, '');
onChange(event);
Expand Down Expand Up @@ -241,11 +243,11 @@ type FormErrorRenderProps = Pick<ClerkElementsError, 'code' | 'message'>;
type FormErrorProps<T> = Omit<T, 'asChild' | 'children'> &
(
| {
children?: (error: FormErrorRenderProps) => React.ReactNode;
children?: (error: FormErrorRenderProps) => ReactNode;
code?: string;
}
| {
children: React.ReactNode;
children: ReactNode;
code: string;
}
);
Expand Down Expand Up @@ -303,9 +305,9 @@ export { Field, FieldError, FieldState, Form, GlobalError, Input, Label, Submit
export type {
FormControlProps,
FormErrorProps,
FormGlobalErrorProps,
FormErrorRenderProps,
FormFieldErrorProps,
FormFieldProps,
FormGlobalErrorProps,
FormProps,
};
7 changes: 3 additions & 4 deletions packages/elements/src/react/common/form/otp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { FormControlProps } from '@radix-ui/react-form';
import { Control as RadixControl } from '@radix-ui/react-form';
import { Slot } from '@radix-ui/react-slot';
import type { CSSProperties, ReactNode, RefObject } from 'react';
import { forwardRef, useImperativeHandle, useLayoutEffect, useRef, useState } from 'react';
import { forwardRef, Fragment, useImperativeHandle, useLayoutEffect, useRef, useState } from 'react';

export type OTPInputProps = Exclude<
FormControlProps,
Expand Down Expand Up @@ -101,7 +100,7 @@ const OTPInputSegmented = forwardRef<HTMLInputElement, Required<Pick<OTPInputPro
}}
>
{Array.from({ length: maxLength }).map((_, i) => (
<Slot key={i}>
<Fragment key={`${String(props.value)[i]}-i`}>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes key error

{render({
value: String(props.value)[i] || '',
status:
Expand All @@ -112,7 +111,7 @@ const OTPInputSegmented = forwardRef<HTMLInputElement, Required<Pick<OTPInputPro
: 'none',
index: i,
})}
</Slot>
</Fragment>
))}
</div>
</div>
Expand Down
Loading
Loading