diff --git a/src/components/Dialog/__tests__/DialogsManager.test.ts b/src/components/Dialog/__tests__/DialogsManager.test.ts index 9a872d284..ead8b887f 100644 --- a/src/components/Dialog/__tests__/DialogsManager.test.ts +++ b/src/components/Dialog/__tests__/DialogsManager.test.ts @@ -222,6 +222,22 @@ describe('DialogManager', () => { expect(Object.keys(dialogManager.state.getLatestValue().dialogsById)).toHaveLength(0); }); + it('removes a dialog from the open stack as soon as it is marked for removal', () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + + const dialogManager = new DialogManager(); + dialogManager.open({ id: 'underlying-dialog' }); + dialogManager.open({ id: dialogId }); + dialogManager.markForRemoval(dialogId); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([ + 'underlying-dialog', + ]); + expect(dialogManager.openDialogCount).toBe(1); + + vi.runAllTimers(); + }); + it('cancels dialog removal if it is referenced again quickly', () => { vi.useFakeTimers({ shouldAdvanceTime: true }); @@ -236,4 +252,26 @@ describe('DialogManager', () => { expect(dialogManager.openDialogCount).toBe(1); expect(Object.keys(dialogManager.state.getLatestValue().dialogsById)).toHaveLength(1); }); + + it('restores an open dialog to the stack when pending removal is cancelled', () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + + const dialogManager = new DialogManager(); + dialogManager.open({ id: 'underlying-dialog' }); + dialogManager.open({ id: dialogId }); + dialogManager.markForRemoval(dialogId); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([ + 'underlying-dialog', + ]); + + dialogManager.getOrCreate({ id: dialogId }); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([ + 'underlying-dialog', + dialogId, + ]); + + vi.runAllTimers(); + }); }); diff --git a/src/components/Dialog/service/DialogManager.ts b/src/components/Dialog/service/DialogManager.ts index 0d984821d..8157d6725 100644 --- a/src/components/Dialog/service/DialogManager.ts +++ b/src/components/Dialog/service/DialogManager.ts @@ -112,6 +112,9 @@ export class DialogManager { removalTimeout: undefined, }, }, + openedDialogIds: current.dialogsById[id]?.isOpen + ? [...current.openedDialogIds.filter((dialogId) => dialogId !== id), id] + : current.openedDialogIds, })); } @@ -200,6 +203,7 @@ export class DialogManager { }, 16), }, }, + openedDialogIds: current.openedDialogIds.filter((dialogId) => dialogId !== id), })); } @@ -221,6 +225,9 @@ export class DialogManager { removalTimeout: undefined, }, }, + openedDialogIds: current.dialogsById[id]?.isOpen + ? [...current.openedDialogIds.filter((dialogId) => dialogId !== id), id] + : current.openedDialogIds, })); } } diff --git a/src/components/Modal/GlobalModal.tsx b/src/components/Modal/GlobalModal.tsx index df5af0f33..b7bfff03d 100644 --- a/src/components/Modal/GlobalModal.tsx +++ b/src/components/Modal/GlobalModal.tsx @@ -126,11 +126,15 @@ export const GlobalModal = ({ maybeClose('escape', event); }; - // Sync open prop → dialog open. Don't close here (dialog ref changes after close → effect loop). + // Sync open prop to dialog state. // closingRef blocks re-open when we just closed and parent hasn't set open=false yet. useEffect(() => { if (!open) { closingRef.current = false; + if (isOpen) { + dialog.close(); + } + return; } if (open && !isOpen && !closingRef.current) { dialog.open(); @@ -162,7 +166,7 @@ export const GlobalModal = ({ inert={isTopmost ? undefined : true} onKeyDown={handleDialogKeyDown} role={role} - tabIndex={-1} + tabIndex={isTopmost ? 0 : -1} > {children} diff --git a/src/components/Modal/__tests__/GlobalModal.test.tsx b/src/components/Modal/__tests__/GlobalModal.test.tsx index dad65a004..a0d54eab3 100644 --- a/src/components/Modal/__tests__/GlobalModal.test.tsx +++ b/src/components/Modal/__tests__/GlobalModal.test.tsx @@ -75,6 +75,54 @@ const renderStackedModals = ({ , ); +const RemovableChildModalFixture = () => { + const [showChild, setShowChild] = React.useState(true); + + return ( + + + + + + {showChild && ( + + + + + + )} + + + + + ); +}; + +const CloseChildModalFixture = () => { + const [childOpen, setChildOpen] = React.useState(true); + + return ( + + + + + + + + + + + + + + + ); +}; + const OverlayCloseButton = React.forwardRef< HTMLButtonElement, React.ComponentProps<'button'> @@ -302,7 +350,7 @@ describe('GlobalModal', () => { const dialog = screen.getByRole('dialog'); expect(dialog).toHaveClass('str-chat__modal__dialog'); expect(dialog).toHaveAttribute('aria-modal', 'true'); - expect(dialog).toHaveAttribute('tabindex', '-1'); + expect(dialog).toHaveAttribute('tabindex', '0'); expect(dialog).toHaveAttribute('aria-labelledby', 'modal-title'); expect(dialog).toHaveAttribute('aria-describedby', 'modal-description'); }); @@ -390,9 +438,11 @@ describe('GlobalModal', () => { expect(parentModal).toBeInTheDocument(); expect(parentModal).not.toHaveAttribute('aria-modal'); expect(parentModal).toHaveAttribute('inert'); + expect(parentModal).toHaveAttribute('tabindex', '-1'); expect(childModal).toBeInTheDocument(); expect(childModal).toHaveAttribute('aria-modal', 'true'); expect(childModal).not.toHaveAttribute('inert'); + expect(childModal).toHaveAttribute('tabindex', '0'); }); it('only closes the topmost modal on Escape', () => { @@ -413,6 +463,59 @@ describe('GlobalModal', () => { expect(parentOnClose).not.toHaveBeenCalled(); }); + it('restores interactivity to the underlying modal after the topmost modal closes', () => { + const childOnClose = vi.fn(); + const parentOnClose = vi.fn(); + + renderStackedModals({ childOnClose, parentOnClose }); + + const parentModal = screen.getByRole('dialog', { name: 'Parent modal' }); + const childModal = screen.getByRole('alertdialog', { name: 'Child modal' }); + + fireEvent.keyDown(childModal, { key: 'Escape' }); + + expect(childOnClose).toHaveBeenCalledTimes(1); + expect(parentModal).toHaveAttribute('aria-modal', 'true'); + expect(parentModal).not.toHaveAttribute('inert'); + expect(parentModal).toHaveAttribute('tabindex', '0'); + + fireEvent.keyDown(parentModal, { key: 'Escape' }); + + expect(parentOnClose).toHaveBeenCalledTimes(1); + }); + + it('restores topmost state to the underlying modal after the topmost modal is removed', () => { + render(); + + const parentModal = screen.getByRole('dialog', { name: 'Parent modal' }); + expect(parentModal).toHaveAttribute('tabindex', '-1'); + + fireEvent.click(screen.getByRole('button', { name: 'Remove child modal' })); + + expect( + screen.queryByRole('alertdialog', { name: 'Child modal' }), + ).not.toBeInTheDocument(); + expect(parentModal).toHaveAttribute('aria-modal', 'true'); + expect(parentModal).not.toHaveAttribute('inert'); + expect(parentModal).toHaveAttribute('tabindex', '0'); + }); + + it('restores topmost state to the underlying modal after the topmost modal open prop becomes false', () => { + render(); + + const parentModal = screen.getByRole('dialog', { name: 'Parent modal' }); + expect(parentModal).toHaveAttribute('tabindex', '-1'); + + fireEvent.click(screen.getByRole('button', { name: 'Close child modal' })); + + expect( + screen.queryByRole('alertdialog', { name: 'Child modal' }), + ).not.toBeInTheDocument(); + expect(parentModal).toHaveAttribute('aria-modal', 'true'); + expect(parentModal).not.toHaveAttribute('inert'); + expect(parentModal).toHaveAttribute('tabindex', '0'); + }); + it('forwards alertdialog role when explicitly provided', () => { renderComponent({ props: {