Repository navigation
feat(mosaic): hold the backup codes menu open while regeneration starts #10436
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
c566a90
f1726fb
31c7b55
29961d8
347eec9
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,2 @@ | ||
| --- | ||
| --- | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { useState } from 'react'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { Pen } from '../../icons/glyphs/pen'; | ||
|
|
@@ -17,6 +18,27 @@ function renderMenu(actions: Parameters<typeof ActionMenu>[0]['actions']) { | |
| ); | ||
| } | ||
|
|
||
| function AsyncMenu({ onRemove = () => undefined }: { onRemove?: () => void }) { | ||
| const [status, setStatus] = useState<'idle' | 'pending'>('idle'); | ||
| return ( | ||
| <MosaicProvider> | ||
| <ActionMenu | ||
| label='Manage passkey' | ||
| actions={[ | ||
| { label: 'Regenerate', status, onClick: () => setStatus('pending') }, | ||
| { label: 'Remove', onClick: onRemove }, | ||
| ]} | ||
| /> | ||
| <button | ||
| type='button' | ||
| onClick={() => setStatus('idle')} | ||
| > | ||
| Finish request | ||
| </button> | ||
| </MosaicProvider> | ||
| ); | ||
| } | ||
|
|
||
| describe('Mosaic ActionMenu', () => { | ||
| it('renders nothing when there are no actions', () => { | ||
| renderMenu([]); | ||
|
|
@@ -37,6 +59,66 @@ describe('Mosaic ActionMenu', () => { | |
|
|
||
| expect(onClick).toHaveBeenCalledOnce(); | ||
| }); | ||
| it('closes after an action that does not report pending', async () => { | ||
|
Member
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. The pending behavior itself has no tests at the
Member
Author
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. Addressed in ed9470f0ba. Added ActionMenu tests for holding open, blocking Escape and outside clicks, disabling sibling actions, and closing when pending clears. Also covered reserved indicator space, spinner timing, and immediate handoff. |
||
| const onClick = vi.fn(); | ||
| const user = userEvent.setup(); | ||
| renderMenu([{ label: 'Make default', onClick }]); | ||
|
|
||
| await user.click(screen.getByRole('button', { name: 'Manage passkey' })); | ||
| await user.click(await screen.findByRole('menuitem', { name: 'Make default' })); | ||
|
|
||
| expect(onClick).toHaveBeenCalledOnce(); | ||
| await waitFor(() => expect(screen.queryByRole('menu')).not.toBeInTheDocument()); | ||
| }); | ||
|
|
||
| it('keeps an opted-in action open while pending and closes immediately when it settles', async () => { | ||
| const user = userEvent.setup(); | ||
| const onRemove = vi.fn(); | ||
| render(<AsyncMenu onRemove={onRemove} />); | ||
| await user.click(screen.getByRole('button', { name: 'Manage passkey' })); | ||
| await user.click(await screen.findByRole('menuitem', { name: 'Regenerate' })); | ||
| const menu = screen.getByRole('menu'); | ||
| expect(within(menu).getByRole('menuitem', { name: 'Regenerate' })).toHaveAttribute('aria-busy', 'true'); | ||
| expect(within(menu).getByRole('menuitem', { name: 'Remove' })).toHaveAttribute('aria-disabled', 'true'); | ||
| await user.click(within(menu).getByRole('menuitem', { name: 'Remove' })); | ||
| expect(onRemove).not.toHaveBeenCalled(); | ||
| await user.keyboard('{Escape}'); | ||
| expect(screen.getByRole('menu')).toBeInTheDocument(); | ||
| await user.click(document.body); | ||
| expect(screen.getByRole('menu')).toBeInTheDocument(); | ||
| await user.click(screen.getByRole('button', { name: 'Finish request' })); | ||
| await waitFor(() => expect(screen.queryByRole('menu')).not.toBeInTheDocument()); | ||
| }); | ||
|
|
||
| it('reserves an indicator immediately and delays its named spinner', async () => { | ||
| const user = userEvent.setup(); | ||
| render(<AsyncMenu />); | ||
| await user.click(screen.getByRole('button', { name: 'Manage passkey' })); | ||
| const item = await screen.findByRole('menuitem', { name: 'Regenerate' }); | ||
| expect(item.querySelector('.cl-menu-indicator')).toBeInTheDocument(); | ||
| vi.useFakeTimers(); | ||
| try { | ||
| fireEvent.click(item); | ||
| expect(screen.queryByRole('progressbar', { name: 'Regenerate' })).not.toBeInTheDocument(); | ||
| await act(() => vi.advanceTimersByTime(149)); | ||
| expect(screen.queryByRole('progressbar', { name: 'Regenerate' })).not.toBeInTheDocument(); | ||
| await act(() => vi.advanceTimersByTime(1)); | ||
| expect(screen.getByRole('progressbar', { name: 'Regenerate' })).toBeInTheDocument(); | ||
| fireEvent.click(screen.getByRole('button', { name: 'Finish request' })); | ||
| expect(screen.queryByRole('menu')).not.toBeInTheDocument(); | ||
| } finally { | ||
| vi.useRealTimers(); | ||
| } | ||
| }); | ||
|
|
||
| it('closes an opted-in action that hands off before becoming pending', async () => { | ||
| const user = userEvent.setup(); | ||
| renderMenu([{ label: 'Open dialog', status: 'idle', onClick: () => undefined }]); | ||
| await user.click(screen.getByRole('button', { name: 'Manage passkey' })); | ||
| await user.click(await screen.findByRole('menuitem', { name: 'Open dialog' })); | ||
| await waitFor(() => expect(screen.queryByRole('menu')).not.toBeInTheDocument()); | ||
| }); | ||
|
|
||
| it('keeps a disabled trigger focusable without allowing pointer or keyboard activation', async () => { | ||
| const onClick = vi.fn(); | ||
| render( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,17 @@ | ||
| import type { Ref } from 'react'; | ||
| import { type Ref, useState } from 'react'; | ||
|
|
||
| import { useSpinDelay } from '../../hooks/use-spin-delay'; | ||
| import type { IconComponent } from '../../icons/glyph'; | ||
| import { Icon } from '../icon'; | ||
| import { Menu } from '../menu'; | ||
| import { Spinner } from '../spinner'; | ||
|
|
||
| export interface ActionMenuAction { | ||
| label: string; | ||
| color?: 'neutral' | 'negative'; | ||
| icon?: IconComponent; | ||
| onClick: () => void; | ||
| status?: 'idle' | 'pending'; | ||
| } | ||
|
|
||
| export interface ActionMenuProps { | ||
|
|
@@ -20,12 +23,30 @@ export interface ActionMenuProps { | |
| } | ||
|
|
||
| export function ActionMenu({ label, actions, triggerRef, disabled, focusableWhenDisabled }: ActionMenuProps) { | ||
| const [open, setOpen] = useState(false); | ||
| const [activated, setActivated] = useState(false); | ||
| const pending = actions.some(action => action.status === 'pending'); | ||
| const showSpinner = useSpinDelay(pending ? true : null, { minDuration: 0 }); | ||
|
|
||
| if (activated && !pending) { | ||
| setActivated(false); | ||
| setOpen(false); | ||
| } | ||
|
|
||
| if (actions.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <Menu.Root placement='bottom-end'> | ||
| <Menu.Root | ||
| placement='bottom-end' | ||
| open={open} | ||
| onOpenChange={nextOpen => { | ||
| if (!pending) { | ||
| setOpen(nextOpen); | ||
| } | ||
| }} | ||
| > | ||
| <Menu.Trigger | ||
| ref={triggerRef} | ||
| aria-label={label} | ||
|
|
@@ -38,15 +59,32 @@ export function ActionMenu({ label, actions, triggerRef, disabled, focusableWhen | |
| key={action.label} | ||
| color={action.color} | ||
| label={action.label} | ||
| onClick={action.onClick} | ||
| disabled={disabled} | ||
| onClick={() => { | ||
| if (action.status !== undefined) { | ||
| setActivated(true); | ||
| } | ||
| action.onClick(); | ||
| }} | ||
| closeOnClick={action.status === undefined} | ||
| aria-busy={action.status === 'pending' || undefined} | ||
| disabled={disabled || pending} | ||
| > | ||
| {action.icon ? ( | ||
| <Menu.Media> | ||
| <Icon glyph={action.icon} /> | ||
| </Menu.Media> | ||
| ) : null} | ||
| <Menu.Label>{action.label}</Menu.Label> | ||
| {action.status !== undefined ? ( | ||
| <Menu.Indicator> | ||
| {action.status === 'pending' && showSpinner ? ( | ||
| <Spinner | ||
| size='sm' | ||
| label={action.label} | ||
| /> | ||
| ) : null} | ||
| </Menu.Indicator> | ||
|
Comment on lines
75
to
+86
Contributor
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.
|
||
| ) : null} | ||
| </Menu.Item> | ||
| ))} | ||
| </Menu.Popup> | ||
|
|
||
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.
Script executed:
Repository: clerk/javascript
Length of output: 5641
Script executed:
Repository: clerk/javascript
Length of output: 41686
Script executed:
Repository: clerk/javascript
Length of output: 41496
Add a release changeset for
@clerk/mosaic.This PR changes user-facing behavior in the publishable
@clerk/mosaicpackage. The empty changeset does not request a version bump or changelog entry, so this fix can be omitted from the next published release.Suggested changeset
Committable suggestion