Skip to content

Commit

Permalink
address pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
panteliselef committed Feb 10, 2025
1 parent 3805d01 commit 6c68ec4
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 19 deletions.
5 changes: 5 additions & 0 deletions integration/testUtils/signInPageObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ export const createSignInComponentPageObject = (testArgs: TestArgs) => {
waitForMounted: (selector = '.cl-signIn-root') => {
return page.waitForSelector(selector, { state: 'attached' });
},
waitForModal: (state?: 'open' | 'closed') => {
return page.waitForSelector('.cl-modalContent:has(.cl-signIn-root)', {
state: state === 'closed' ? 'detached' : 'attached',
});
},
setIdentifier: (val: string) => {
return self.getIdentifierInput().fill(val);
},
Expand Down
5 changes: 5 additions & 0 deletions integration/testUtils/signUpPageObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export const createSignUpComponentPageObject = (testArgs: TestArgs) => {
waitForMounted: (selector = '.cl-signUp-root') => {
return page.waitForSelector(selector, { state: 'attached' });
},
waitForModal: (state?: 'open' | 'closed') => {
return page.waitForSelector('.cl-modalContent:has(.cl-signUp-root)', {
state: state === 'closed' ? 'detached' : 'attached',
});
},
signUpWithOauth: (provider: string) => {
return page.getByRole('button', { name: new RegExp(`continue with ${provider}`, 'gi') });
},
Expand Down
8 changes: 4 additions & 4 deletions integration/tests/oauth-flows.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { test } from '@playwright/test';
import { createClerkClient } from '@clerk/backend';
import { test } from '@playwright/test';

import { appConfigs } from '../presets';
import { instanceKeys } from '../presets/envs';
import type { FakeUser } from '../testUtils';
import { createTestUtils, testAgainstRunningApps } from '../testUtils';
import { instanceKeys } from '../presets/envs';
import { createUserService } from '../testUtils/usersService';

testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flows @nextjs', ({ app }) => {
Expand Down Expand Up @@ -78,7 +78,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo

await u.page.getByText('Sign in button (force)').click();

await u.po.signIn.waitForMounted();
await u.po.signIn.waitForModal();
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
await u.page.getByText('Sign in to oauth-provider').waitFor();

Expand All @@ -103,7 +103,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo

await u.page.getByText('Sign up button (force)').click();

await u.po.signUp.waitForMounted();
await u.po.signUp.waitForModal();
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
await u.page.getByText('Sign in to oauth-provider').waitFor();

Expand Down
10 changes: 10 additions & 0 deletions integration/tests/sign-in-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign in f
await u.po.expect.toBeSignedIn();
});

test('(modal) sign in with email and instant password', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
await u.page.goToRelative('/buttons');
await u.page.getByText('Sign in button (force)').click();
await u.po.signIn.waitForModal();
await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
await u.po.expect.toBeSignedIn();
await u.po.signIn.waitForModal('closed');
});

test('sign in with email code', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
await u.po.signIn.goTo();
Expand Down
43 changes: 43 additions & 0 deletions integration/tests/sign-in-or-up-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign-
await u.po.expect.toBeSignedIn();
});

test('(modal) sign in with email code', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
await u.page.goToRelative('/buttons');
await u.page.getByText('Sign in button (fallback)').click();
await u.po.signIn.waitForModal();
await u.po.signIn.getIdentifierInput().fill(fakeUser.email);
await u.po.signIn.continue();
await u.po.signIn.getUseAnotherMethodLink().click();
await u.po.signIn.getAltMethodsEmailCodeButton().click();
await u.po.signIn.enterTestOtpCode();
await u.po.expect.toBeSignedIn();
await u.po.signIn.waitForModal('closed');
});

test('sign in with phone number and password', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
await u.po.signIn.goTo();
Expand Down Expand Up @@ -221,6 +235,35 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign-
await fakeUser.deleteIfExists();
});

test('(modal) sign up with username, email, and password', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
const fakeUser = u.services.users.createFakeUser({
fictionalEmail: true,
withPassword: true,
withUsername: true,
});

await u.page.goToRelative('/buttons');
await u.page.getByText('Sign in button (fallback)').click();
await u.po.signIn.waitForModal();
await u.po.signIn.setIdentifier(fakeUser.username);
await u.po.signIn.continue();

const prefilledUsername = u.po.signUp.getUsernameInput();
await expect(prefilledUsername).toHaveValue(fakeUser.username);

await u.po.signUp.setEmailAddress(fakeUser.email);
await u.po.signUp.setPassword(fakeUser.password);
await u.po.signUp.continue();

await u.po.signUp.enterTestOtpCode();

await u.po.expect.toBeSignedIn();
await u.po.signIn.waitForModal('closed');

await fakeUser.deleteIfExists();
});

test('sign up, sign out and sign in again', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
const fakeUser = u.services.users.createFakeUser({
Expand Down
33 changes: 32 additions & 1 deletion integration/tests/sign-up-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { appConfigs } from '../presets';
import { createTestUtils, testAgainstRunningApps } from '../testUtils';

testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up flow @generic @nextjs', ({ app }) => {
test.describe.configure({ mode: 'serial' });
test.describe.configure({ mode: 'parallel' });

test.afterAll(async () => {
await app.teardown();
Expand Down Expand Up @@ -90,6 +90,37 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up f
await fakeUser.deleteIfExists();
});

test('(modal) can sign up with phone number', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
const fakeUser = u.services.users.createFakeUser({
fictionalEmail: true,
withPhoneNumber: true,
withUsername: true,
});

// Open modal
await u.page.goToRelative('/buttons');
await u.page.getByText('Sign up button (fallback)').click();
await u.po.signUp.waitForModal();

// Fill in sign up form
await u.po.signUp.signUp({
email: fakeUser.email,
phoneNumber: fakeUser.phoneNumber,
password: fakeUser.password,
});

// Verify email
await u.po.signUp.enterTestOtpCode();
// Verify phone number
await u.po.signUp.enterTestOtpCode();

// Check if user is signed in
await u.po.expect.toBeSignedIn();
await u.po.signUp.waitForModal('closed');
await fakeUser.deleteIfExists();
});

test('sign up with first name, last name, email, phone and password', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
const fakeUser = u.services.users.createFakeUser({
Expand Down
18 changes: 10 additions & 8 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export class Clerk implements ClerkInterface {
#loaded = false;

#listeners: Array<(emission: Resources) => void> = [];
#externalNavigationListeners: Array<() => void> = [];
#navigationListeners: Array<() => void> = [];
#options: ClerkOptions = {};
#pageLifecycle: ReturnType<typeof createPageLifecycle> | null = null;
#touchThrottledUntil = 0;
Expand Down Expand Up @@ -962,10 +962,10 @@ export class Clerk implements ClerkInterface {
return unsubscribe;
};

public __internal_externalNavigationListener = (listener: () => void): UnsubscribeCallback => {
this.#externalNavigationListeners.push(listener);
public __internal_addNavigationListener = (listener: () => void): UnsubscribeCallback => {
this.#navigationListeners.push(listener);
const unsubscribe = () => {
this.#externalNavigationListeners = this.#externalNavigationListeners.filter(l => l !== listener);
this.#navigationListeners = this.#navigationListeners.filter(l => l !== listener);
};
return unsubscribe;
};
Expand All @@ -975,9 +975,11 @@ export class Clerk implements ClerkInterface {
return;
}

/**
* Trigger all navigation listeners. In order for modal UI components to close.
*/
setTimeout(() => {
// Notify after the navigation, in order to visit a page with the updated context set above.
this.#notifyExternalNavigationListeners();
this.#emitNavigationListeners();
}, 0);

let toURL = new URL(to, window.location.href);
Expand Down Expand Up @@ -2056,8 +2058,8 @@ export class Clerk implements ClerkInterface {
}
};

#notifyExternalNavigationListeners = (): void => {
for (const listener of this.#externalNavigationListeners) {
#emitNavigationListeners = (): void => {
for (const listener of this.#navigationListeners) {
listener();
}
};
Expand Down
2 changes: 1 addition & 1 deletion packages/clerk-js/src/ui/lazyModules/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type LazyModalRendererProps = React.PropsWithChildren<
flowName?: FlowMetadata['flow'];
startPath?: string;
onClose?: ModalProps['handleClose'];
onExternalNavigate: () => any;
onExternalNavigate: () => void;
modalContainerSx?: ThemableCssProp;
modalContentSx?: ThemableCssProp;
canCloseModal?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/ui/router/VirtualRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const VirtualRouter = ({
onExternalNavigate,
children,
}: VirtualRouterProps): JSX.Element => {
const { __internal_externalNavigationListener } = useClerk();
const { __internal_addNavigationListener } = useClerk();
const [currentURL, setCurrentURL] = React.useState(
new URL('/' + VIRTUAL_ROUTER_BASE_PATH + startPath, window.location.origin),
);
Expand All @@ -27,7 +27,7 @@ export const VirtualRouter = ({
useEffect(() => {
let unsubscribe = () => {};
if (onExternalNavigate) {
unsubscribe = __internal_externalNavigationListener(onExternalNavigate);
unsubscribe = __internal_addNavigationListener(onExternalNavigate);
}
return () => {
unsubscribe();
Expand Down
6 changes: 3 additions & 3 deletions packages/types/src/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,10 @@ export interface Clerk {
addListener: (callback: ListenerCallback) => UnsubscribeCallback;

/**
* Internal function that notifies modal UI components when a navigation event occurs,
* allowing them to close if necessary.
* Registers and internal listener that triggers a callback each time `Clerk.navigate` is called.
* Its purpose is to notify modal UI components when a navigation event occurs, allowing them to close if necessary.
*/
__internal_externalNavigationListener: (callback: () => void) => UnsubscribeCallback;
__internal_addNavigationListener: (callback: () => void) => UnsubscribeCallback;

/**
* Set the active session and organization explicitly.
Expand Down

0 comments on commit 6c68ec4

Please sign in to comment.