From 29d21117fcd48c1b716d51e67b14ac3b97fae92e Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 3 Jun 2026 13:34:04 +0200 Subject: [PATCH 1/5] feat: allow to stack modals on top of each other --- .../__tests__/DialogManagerContext.test.tsx | 78 ++++++++++++++++++- .../Dialog/__tests__/DialogsManager.test.ts | 45 +++++++++++ src/components/Dialog/hooks/useDialog.ts | 27 +++++-- .../Dialog/service/DialogManager.ts | 16 ++-- src/components/Dialog/styling/Alert.scss | 1 + src/components/Modal/GlobalModal.tsx | 20 +++-- .../Modal/__tests__/GlobalModal.test.tsx | 76 +++++++++++++++--- 7 files changed, 229 insertions(+), 34 deletions(-) diff --git a/src/components/Dialog/__tests__/DialogManagerContext.test.tsx b/src/components/Dialog/__tests__/DialogManagerContext.test.tsx index 66edd0f87..22d9d85b5 100644 --- a/src/components/Dialog/__tests__/DialogManagerContext.test.tsx +++ b/src/components/Dialog/__tests__/DialogManagerContext.test.tsx @@ -5,12 +5,13 @@ import { useDialogManager, } from '../../../context/DialogManagerContext'; -import { useDialogIsOpen, useOpenedDialogCount } from '../hooks'; +import { useDialogIsOpen, useDialogIsTopmost, useOpenedDialogCount } from '../hooks'; const TEST_IDS = { CLOSE_DIALOG: 'close-dialog', DIALOG_COUNT: 'dialog-count', DIALOG_OPEN: 'dialog-open', + DIALOG_TOPMOST: 'dialog-topmost', MANAGER_ID_DISPLAY: 'manager-id-display', OPEN_DIALOG: 'open-dialog', TEST_COMPONENT: 'test-component', @@ -21,20 +22,34 @@ const SHARED_MANAGER_ID = 'shared-manager'; const MANAGER_1_ID = 'manager-1'; const MANAGER_2_ID = 'manager-2'; -const TestComponent = ({ dialogId, dialogManagerId, testId }: any) => { +type TestComponentProps = { + dialogId?: string; + dialogManagerId?: string; + testId?: string; +}; + +const TestComponent = ({ dialogId, dialogManagerId, testId }: TestComponentProps) => { const { dialogManager } = useDialogManager({ dialogId, dialogManagerId }); + const selectorDialogId = dialogId ?? 'test-dialog'; const openDialogCount = useOpenedDialogCount({ dialogManagerId }); - const isOpen = useDialogIsOpen(dialogId, dialogManagerId); + const isOpen = useDialogIsOpen(selectorDialogId, dialogManagerId); + const isTopmost = useDialogIsTopmost(selectorDialogId, dialogManagerId); return (
{dialogManager?.id} {openDialogCount} {isOpen ? 'true' : 'false'} + {isTopmost ? 'true' : 'false'}
); }; -const DialogTestComponent = ({ dialogId, managerId }: any) => { +type DialogTestComponentProps = { + dialogId: string; + managerId?: string; +}; + +const DialogTestComponent = ({ dialogId, managerId }: DialogTestComponentProps) => { const { dialogManager } = useDialogManager({ dialogManagerId: managerId }); const handleOpenDialog = () => { @@ -171,6 +186,61 @@ describe('DialogManagerContext', () => { ).toHaveTextContent('true'); }); + it('updates topmost dialog state when the dialog stack changes', () => { + render( + + + + + + , + ); + + const firstComponent = screen.getByTestId('first-component'); + const secondComponent = screen.getByTestId('second-component'); + + act(() => { + fireEvent.click(screen.getAllByTestId(TEST_IDS.OPEN_DIALOG)[0]); + }); + + expect( + firstComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('true'); + expect( + secondComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('false'); + + act(() => { + fireEvent.click(screen.getAllByTestId(TEST_IDS.OPEN_DIALOG)[1]); + }); + + expect( + firstComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('false'); + expect( + secondComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('true'); + + act(() => { + fireEvent.click(screen.getAllByTestId(TEST_IDS.CLOSE_DIALOG)[1]); + }); + + expect( + firstComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('true'); + expect( + secondComponent.querySelector(`[data-testid="${TEST_IDS.DIALOG_TOPMOST}"]`), + ).toHaveTextContent('false'); + }); + it('creates different managers for different IDs', () => { render( diff --git a/src/components/Dialog/__tests__/DialogsManager.test.ts b/src/components/Dialog/__tests__/DialogsManager.test.ts index 3dadd17f6..9a872d284 100644 --- a/src/components/Dialog/__tests__/DialogsManager.test.ts +++ b/src/components/Dialog/__tests__/DialogsManager.test.ts @@ -73,6 +73,40 @@ describe('DialogManager', () => { expect(dialogManager.openDialogCount).toBe(1); }); + it('tracks opened dialogs in stack order', () => { + const dialogManager = new DialogManager(); + + dialogManager.open({ id: 'first-dialog' }); + dialogManager.open({ id: 'second-dialog' }); + dialogManager.open({ id: 'third-dialog' }); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([ + 'first-dialog', + 'second-dialog', + 'third-dialog', + ]); + expect(dialogManager.openDialogCount).toBe(3); + + dialogManager.close('third-dialog'); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([ + 'first-dialog', + 'second-dialog', + ]); + expect(dialogManager.openDialogCount).toBe(2); + }); + + it('keeps only the target dialog in the stack when opening with closeRest', () => { + const dialogManager = new DialogManager(); + + dialogManager.open({ id: 'first-dialog' }); + dialogManager.open({ id: 'second-dialog' }); + dialogManager.open({ id: dialogId }, true); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([dialogId]); + expect(dialogManager.openDialogCount).toBe(1); + }); + it('opens existing dialog', () => { const dialogManager = new DialogManager(); dialogManager.getOrCreate({ id: dialogId }); @@ -154,6 +188,17 @@ describe('DialogManager', () => { expect(Object.keys(dialogManager.state.getLatestValue().dialogsById)).toHaveLength(0); }); + it('removes a dialog from the open stack', () => { + const dialogManager = new DialogManager(); + + dialogManager.open({ id: 'first-dialog' }); + dialogManager.open({ id: dialogId }); + dialogManager.remove('first-dialog'); + + expect(dialogManager.state.getLatestValue().openedDialogIds).toEqual([dialogId]); + expect(dialogManager.openDialogCount).toBe(1); + }); + it('handles attempt to remove non-existent dialog', () => { const dialogManager = new DialogManager(); dialogManager.getOrCreate({ id: dialogId }); diff --git a/src/components/Dialog/hooks/useDialog.ts b/src/components/Dialog/hooks/useDialog.ts index 75a713af4..5e5ae403e 100644 --- a/src/components/Dialog/hooks/useDialog.ts +++ b/src/components/Dialog/hooks/useDialog.ts @@ -49,8 +49,8 @@ export const useDialogOnNearestManager = ({ id }: Pick) = export const modalDialogId = 'modal-dialog' as const; -export const useModalDialog = () => - useDialog({ dialogManagerId: modalDialogManagerId, id: modalDialogId }); +export const useModalDialog = (id: string = modalDialogId) => + useDialog({ dialogManagerId: modalDialogManagerId, id }); export const useDialogIsOpen = (id: string, dialogManagerId?: string) => { const { dialogManager } = useDialogManager({ dialogManagerId }); @@ -61,14 +61,25 @@ export const useDialogIsOpen = (id: string, dialogManagerId?: string) => { return useStateStore(dialogManager.state, dialogIsOpenSelector).isOpen; }; -export const useModalDialogIsOpen = () => - useDialogIsOpen(modalDialogId, modalDialogManagerId); +export const useModalDialogIsOpen = (id: string = modalDialogId) => + useDialogIsOpen(id, modalDialogManagerId); + +export const useDialogIsTopmost = (id: string, dialogManagerId?: string) => { + const { dialogManager } = useDialogManager({ dialogManagerId }); + const dialogIsTopmostSelector = useCallback( + ({ openedDialogIds }: DialogManagerState) => ({ + isTopmost: openedDialogIds[openedDialogIds.length - 1] === id, + }), + [id], + ); + return useStateStore(dialogManager.state, dialogIsTopmostSelector).isTopmost; +}; + +export const useModalDialogIsTopmost = (id: string = modalDialogId) => + useDialogIsTopmost(id, modalDialogManagerId); const openedDialogCountSelector = (nextValue: DialogManagerState) => ({ - openedDialogCount: Object.values(nextValue.dialogsById).reduce((count, dialog) => { - if (dialog.isOpen) return count + 1; - return count; - }, 0), + openedDialogCount: nextValue.openedDialogIds.length, }); export const useOpenedDialogCount = ({ diff --git a/src/components/Dialog/service/DialogManager.ts b/src/components/Dialog/service/DialogManager.ts index 1f11a826c..0d984821d 100644 --- a/src/components/Dialog/service/DialogManager.ts +++ b/src/components/Dialog/service/DialogManager.ts @@ -35,6 +35,7 @@ type Dialogs = Record; export type DialogManagerState = { dialogsById: Dialogs; + openedDialogIds: DialogId[]; }; /** @@ -53,6 +54,7 @@ export class DialogManager { id: string; state = new StateStore({ dialogsById: {}, + openedDialogIds: [], }); constructor({ closeOnClickOutside = true, id }: DialogManagerOptions = {}) { @@ -61,13 +63,7 @@ export class DialogManager { } get openDialogCount() { - return Object.values(this.state.getLatestValue().dialogsById).reduce( - (count, dialog) => { - if (dialog.isOpen) return count + 1; - return count; - }, - 0, - ); + return this.state.getLatestValue().openedDialogIds.length; } get(id: DialogId): Dialog | undefined { @@ -131,6 +127,10 @@ export class DialogManager { this.state.next((current) => ({ ...current, dialogsById: { ...current.dialogsById, [dialog.id]: { ...dialog, isOpen: true } }, + openedDialogIds: [ + ...current.openedDialogIds.filter((dialogId) => dialogId !== dialog.id), + dialog.id, + ], })); } @@ -140,6 +140,7 @@ export class DialogManager { this.state.next((current) => ({ ...current, dialogsById: { ...current.dialogsById, [dialog.id]: { ...dialog, isOpen: false } }, + openedDialogIds: current.openedDialogIds.filter((dialogId) => dialogId !== id), })); } @@ -171,6 +172,7 @@ export class DialogManager { return { ...current, dialogsById: newDialogs, + openedDialogIds: current.openedDialogIds.filter((dialogId) => dialogId !== id), }; }); }; diff --git a/src/components/Dialog/styling/Alert.scss b/src/components/Dialog/styling/Alert.scss index 008abaa4c..24ee97269 100644 --- a/src/components/Dialog/styling/Alert.scss +++ b/src/components/Dialog/styling/Alert.scss @@ -34,6 +34,7 @@ .str-chat__alert-header__description { font: var(--str-chat__font-caption-default); + margin: 0; } } } diff --git a/src/components/Modal/GlobalModal.tsx b/src/components/Modal/GlobalModal.tsx index 03fde62ad..7c96c4ecf 100644 --- a/src/components/Modal/GlobalModal.tsx +++ b/src/components/Modal/GlobalModal.tsx @@ -20,8 +20,10 @@ import { modalDialogId, useModalDialog, useModalDialogIsOpen, + useModalDialogIsTopmost, } from '../Dialog'; import { useResolvedModalAriaProps } from '../../a11y/hooks/useResolvedModalAriaProps'; +import { useStableId } from '../UtilityComponents/useStableId'; export type ModalCloseEvent = | KeyboardEvent @@ -35,6 +37,8 @@ export type ModalProps = { open: boolean; /** Custom class to be applied to the modal root div */ className?: string; + /** Optional stable id for this modal instance. Generated automatically when omitted. */ + dialogId?: string; /** Accessible label for the modal dialog. Ignored when aria-labelledby is provided. */ 'aria-label'?: string; /** ID of the element that labels the modal dialog. */ @@ -58,13 +62,17 @@ export const GlobalModal = ({ children, className, CloseButtonOnOverlay, + dialogId, onClose, onCloseAttempt, open, role = 'dialog', }: PropsWithChildren) => { - const dialog = useModalDialog(); - const isOpen = useModalDialogIsOpen(); + const generatedDialogId = useStableId(); + const resolvedDialogId = dialogId ?? `${modalDialogId}-${generatedDialogId}`; + const dialog = useModalDialog(resolvedDialogId); + const isOpen = useModalDialogIsOpen(resolvedDialogId); + const isTopmost = useModalDialogIsTopmost(resolvedDialogId); const overlayRef = useRef(null); const closeButtonRef = useRef(null); const closingRef = useRef(false); @@ -98,6 +106,7 @@ export const GlobalModal = ({ ); const handleOverlayClick = (event: React.MouseEvent) => { + if (!isTopmost) return; const target = event.target as HTMLDivElement; if (overlayRef.current === target) { maybeClose('overlay', event); @@ -105,11 +114,12 @@ export const GlobalModal = ({ }; const handleCloseButtonClick = (event: React.MouseEvent) => { + if (!isTopmost) return; maybeClose('button', event); }; const handleDialogKeyDown = (event: React.KeyboardEvent) => { - if (event.defaultPrevented || event.key !== 'Escape') return; + if (event.defaultPrevented || event.key !== 'Escape' || !isTopmost) return; maybeClose('escape', event); }; @@ -127,7 +137,7 @@ export const GlobalModal = ({ if (!open || !isOpen) return null; return ( - +
- +
, ); +const renderStackedModals = ({ + childOnClose = vi.fn(), + parentOnClose = vi.fn(), +}: { + childOnClose?: () => void; + parentOnClose?: () => void; +} = {}) => + render( + + + + + + + + + + , + ); + // Wrap children in a focusable element so FocusScope can manage focus const ModalContent = ({ children, @@ -253,15 +278,16 @@ describe('GlobalModal', () => { props: { children: ( - +

Autogenerated title target

 ), + dialogId: 'custom-modal', open: true, }, }); const dialog = screen.getByRole('dialog', { name: 'Autogenerated title target' }); - expect(dialog).toHaveAttribute('aria-labelledby', 'modal-dialog-title'); + expect(dialog).toHaveAttribute('aria-labelledby', 'custom-modal-title'); expect(dialog).not.toHaveAttribute('aria-label'); }); @@ -270,17 +296,18 @@ describe('GlobalModal', () => { props: { children: ( - - +

Create poll

+

Create poll description

 ), + dialogId: 'custom-modal', open: true, }, }); const dialog = screen.getByRole('dialog', { name: 'Create poll' }); - expect(dialog).toHaveAttribute('aria-labelledby', 'modal-dialog-title'); - expect(dialog).toHaveAttribute('aria-describedby', 'modal-dialog-description'); + expect(dialog).toHaveAttribute('aria-labelledby', 'custom-modal-title'); + expect(dialog).toHaveAttribute('aria-describedby', 'custom-modal-description'); }); it('autowires aria-describedby for alertdialog when not explicitly provided', () => { @@ -288,18 +315,47 @@ describe('GlobalModal', () => { props: { children: ( - - +

Delete message

+

Are you sure?

 ), + dialogId: 'delete-message-alert', open: true, role: 'alertdialog', }, }); const dialog = screen.getByRole('alertdialog', { name: 'Delete message' }); - expect(dialog).toHaveAttribute('aria-labelledby', 'modal-dialog-title'); - expect(dialog).toHaveAttribute('aria-describedby', 'modal-dialog-description'); + expect(dialog).toHaveAttribute('aria-labelledby', 'delete-message-alert-title'); + expect(dialog).toHaveAttribute( + 'aria-describedby', + 'delete-message-alert-description', + ); + }); + + it('supports rendering an alertdialog above an existing modal', () => { + renderStackedModals(); + + expect(screen.getByRole('dialog', { name: 'Parent modal' })).toBeInTheDocument(); + expect(screen.getByRole('alertdialog', { name: 'Child modal' })).toBeInTheDocument(); + }); + + it('only closes the topmost modal on Escape', () => { + const childOnClose = vi.fn(); + const parentOnClose = vi.fn(); + + renderStackedModals({ childOnClose, parentOnClose }); + + fireEvent.keyDown(screen.getByRole('dialog', { name: 'Parent modal' }), { + key: 'Escape', + }); + expect(parentOnClose).not.toHaveBeenCalled(); + + fireEvent.keyDown(screen.getByRole('alertdialog', { name: 'Child modal' }), { + key: 'Escape', + }); + expect(childOnClose).toHaveBeenCalledTimes(1); + expect(parentOnClose).not.toHaveBeenCalled(); }); it('forwards alertdialog role when explicitly provided', () => { From c5ea8f81b4c7177a4ab36ac6278e0c020d1b3e46 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 3 Jun 2026 14:03:47 +0200 Subject: [PATCH 2/5] test: fix tests --- .../Message/__tests__/MessageText.test.tsx | 15 +- .../__snapshots__/MessageText.test.tsx.snap | 220 ------------------ .../__tests__/MessageActions.test.tsx | 15 +- .../__tests__/AttachmentSelector.test.tsx | 22 +- 4 files changed, 30 insertions(+), 242 deletions(-) delete mode 100644 src/components/Message/__tests__/__snapshots__/MessageText.test.tsx.snap diff --git a/src/components/Message/__tests__/MessageText.test.tsx b/src/components/Message/__tests__/MessageText.test.tsx index 8cea758c1..7bc88a92f 100644 --- a/src/components/Message/__tests__/MessageText.test.tsx +++ b/src/components/Message/__tests__/MessageText.test.tsx @@ -418,26 +418,29 @@ describe('', () => { it('should render with a custom wrapper class when one is set', async () => { const customWrapperClass = 'custom-wrapper'; const message = generateMessage({ text: 'hello world' }); - const { container } = await renderMessageText({ + const { getByText } = await renderMessageText({ customProps: { customWrapperClass, message }, }); - expect(container).toMatchSnapshot(); + + expect(getByText('hello world')).toBeInTheDocument(); }); it('should render with a custom inner class when one is set', async () => { const customInnerClass = 'custom-inner'; const message = generateMessage({ text: 'hi mate' }); - const { container } = await renderMessageText({ + const { getByText } = await renderMessageText({ customProps: { customInnerClass, message }, }); - expect(container).toMatchSnapshot(); + + expect(getByText('hi mate')).toBeInTheDocument(); }); it('should render with custom theme identifier in generated css classes when theme is set', async () => { const message = generateMessage({ text: 'whatup?!' }); - const { container } = await renderMessageText({ + const { getByText } = await renderMessageText({ customProps: { message, theme: 'custom' }, }); - expect(container).toMatchSnapshot(); + + expect(getByText('whatup?!')).toBeInTheDocument(); }); }); diff --git a/src/components/Message/__tests__/__snapshots__/MessageText.test.tsx.snap b/src/components/Message/__tests__/__snapshots__/MessageText.test.tsx.snap deleted file mode 100644 index 235f1a55c..000000000 --- a/src/components/Message/__tests__/__snapshots__/MessageText.test.tsx.snap +++ /dev/null @@ -1,220 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[` > should render with a custom inner class when one is set 1`] = ` -
-
-
-
-
-
- - aria/Message, - -
-
-

- hi mate -

-
-
-
-
-
-
-
- -
-
-
-
-
-
-`; - -exports[` > should render with a custom wrapper class when one is set 1`] = ` -
-
-
-
-
-
- - aria/Message, - -
-
-

- hello world -

-
-
-
-
-
-
-
- -
-
-
-
-
-
-`; - -exports[` > should render with custom theme identifier in generated css classes when theme is set 1`] = ` -
-
-
-
-
-
- - aria/Message, - -
-
-

- whatup?! -

-
-
-
-
-
-
-
- -
-
-
-
-
-
-`; diff --git a/src/components/MessageActions/__tests__/MessageActions.test.tsx b/src/components/MessageActions/__tests__/MessageActions.test.tsx index 658fa109d..9c8c19172 100644 --- a/src/components/MessageActions/__tests__/MessageActions.test.tsx +++ b/src/components/MessageActions/__tests__/MessageActions.test.tsx @@ -344,11 +344,16 @@ describe('', () => { }); const dialog = screen.getByRole('alertdialog', { name: 'Delete message' }); - expect(dialog).toHaveAttribute('aria-labelledby', 'modal-dialog-title'); - expect(dialog).toHaveAttribute('aria-describedby', 'modal-dialog-description'); - expect( - screen.getByText('Are you sure you want to delete this message?'), - ).toHaveAttribute('id', 'modal-dialog-description'); + const labelledBy = dialog.getAttribute('aria-labelledby'); + const describedBy = dialog.getAttribute('aria-describedby'); + expect(labelledBy).toBeTruthy(); + expect(describedBy).toBeTruthy(); + expect(document.getElementById(labelledBy ?? '')).toHaveTextContent( + 'Delete message', + ); + expect(document.getElementById(describedBy ?? '')).toHaveTextContent( + 'Are you sure you want to delete this message?', + ); }); it('should include Edit in dropdown actions when user has edit capability', async () => { diff --git a/src/components/MessageComposer/__tests__/AttachmentSelector.test.tsx b/src/components/MessageComposer/__tests__/AttachmentSelector.test.tsx index 4088bd157..cae686953 100644 --- a/src/components/MessageComposer/__tests__/AttachmentSelector.test.tsx +++ b/src/components/MessageComposer/__tests__/AttachmentSelector.test.tsx @@ -47,8 +47,9 @@ const SIMPLE_ATTACHMENT_SELECTOR_TEST_ID = 'invoke-attachment-selector-button'; const UPLOAD_INPUT_TEST_ID = 'file-input'; const translationContext = fromPartial({ - t: (v: any) => v, - tDateTimeParser: (v: any) => v.toString(), + t: ((value: string) => value) as TranslationContextValue['t'], + tDateTimeParser: ((value: string) => + value.toString()) as TranslationContextValue['tDateTimeParser'], }); const defaultChannelData = { @@ -488,13 +489,14 @@ describe('AttachmentSelector', () => { }); const dialog = screen.getByRole('dialog', { name: 'Create poll' }); - expect(dialog).toHaveAttribute('aria-describedby', 'modal-dialog-description'); - expect(document.getElementById('modal-dialog-description')).toHaveTextContent( + const descriptionId = dialog.getAttribute('aria-describedby'); + expect(descriptionId).toBeTruthy(); + expect(document.getElementById(descriptionId ?? '')).toHaveTextContent( 'Create a question, add options, and configure poll settings', ); expect(screen.getByPlaceholderText(/Ask a question/i)).toHaveAttribute( 'aria-describedby', - expect.stringContaining('modal-dialog-description'), + expect.stringContaining(descriptionId ?? ''), ); const invokeButtonFocusSpy = vi.spyOn(invokeButton, 'focus'); @@ -528,17 +530,15 @@ describe('AttachmentSelector', () => { }); const dialog = screen.getByRole('dialog', { name: /share location/i }); - expect(dialog).toHaveAttribute('aria-describedby', 'modal-dialog-description'); - expect(document.getElementById('modal-dialog-description')).toHaveTextContent( + const descriptionId = dialog.getAttribute('aria-describedby'); + expect(descriptionId).toBeTruthy(); + expect(document.getElementById(descriptionId ?? '')).toHaveTextContent( 'Select your current location and optionally enable live location sharing', ); const closePromptButton = document.querySelector( '.str-chat__prompt__header__close-button', ) as HTMLButtonElement | null; - expect(closePromptButton).toHaveAttribute( - 'aria-describedby', - 'modal-dialog-description', - ); + expect(closePromptButton).toHaveAttribute('aria-describedby', descriptionId); const invokeButtonFocusSpy = vi.spyOn(invokeButton, 'focus'); fireEvent.keyDown(dialog, { key: 'Escape' }); From f811b407af2248fbaf07e314f68183a74c8b3050 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 3 Jun 2026 15:03:12 +0200 Subject: [PATCH 3/5] fix: address code review remarks --- .../Message/__tests__/MessageText.test.tsx | 30 ++++++++---- src/components/Modal/GlobalModal.tsx | 3 +- .../Modal/__tests__/GlobalModal.test.tsx | 46 +++++++++++-------- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/components/Message/__tests__/MessageText.test.tsx b/src/components/Message/__tests__/MessageText.test.tsx index 7bc88a92f..6c9d45942 100644 --- a/src/components/Message/__tests__/MessageText.test.tsx +++ b/src/components/Message/__tests__/MessageText.test.tsx @@ -419,28 +419,42 @@ describe('', () => { const customWrapperClass = 'custom-wrapper'; const message = generateMessage({ text: 'hello world' }); const { getByText } = await renderMessageText({ - customProps: { customWrapperClass, message }, + customProps: { + message, + Message: () => ( + + ), + }, }); - expect(getByText('hello world')).toBeInTheDocument(); + expect(getByText('hello world').closest(`.${customWrapperClass}`)).toHaveClass( + customWrapperClass, + ); }); it('should render with a custom inner class when one is set', async () => { const customInnerClass = 'custom-inner'; const message = generateMessage({ text: 'hi mate' }); - const { getByText } = await renderMessageText({ - customProps: { customInnerClass, message }, + const { getByTestId } = await renderMessageText({ + customProps: { + message, + Message: () => ( + + ), + }, }); - expect(getByText('hi mate')).toBeInTheDocument(); + expect(getByTestId(messageTextTestId)).toHaveClass(customInnerClass); }); - it('should render with custom theme identifier in generated css classes when theme is set', async () => { + it('should render with the default wrapper class when no custom wrapper class is set', async () => { const message = generateMessage({ text: 'whatup?!' }); const { getByText } = await renderMessageText({ - customProps: { message, theme: 'custom' }, + customProps: { message }, }); - expect(getByText('whatup?!')).toBeInTheDocument(); + expect(getByText('whatup?!').closest('.str-chat__message-text')).toHaveClass( + 'str-chat__message-text', + ); }); }); diff --git a/src/components/Modal/GlobalModal.tsx b/src/components/Modal/GlobalModal.tsx index 7c96c4ecf..355791cdf 100644 --- a/src/components/Modal/GlobalModal.tsx +++ b/src/components/Modal/GlobalModal.tsx @@ -154,8 +154,9 @@ export const GlobalModal = ({ aria-describedby={resolvedModalAriaProps['aria-describedby']} aria-label={resolvedModalAriaProps['aria-label']} aria-labelledby={resolvedModalAriaProps['aria-labelledby']} - aria-modal='true' + aria-modal={isTopmost ? 'true' : undefined} className='str-chat__modal__dialog' + inert={isTopmost ? undefined : true} onKeyDown={handleDialogKeyDown} role={role} tabIndex={-1} diff --git a/src/components/Modal/__tests__/GlobalModal.test.tsx b/src/components/Modal/__tests__/GlobalModal.test.tsx index 0ed9d67ab..f57de175d 100644 --- a/src/components/Modal/__tests__/GlobalModal.test.tsx +++ b/src/components/Modal/__tests__/GlobalModal.test.tsx @@ -7,15 +7,32 @@ import { mockChatContext } from '../../../mock-builders'; import { axe } from '../../../../axe-helper'; const OVERLAY_SELECTOR = '.str-chat__modal'; -const renderComponent = ({ props }: any = {}) => + +type GlobalModalProps = React.ComponentProps; + +const renderComponent = ({ props }: { props?: Partial } = {}) => render( - + , ); +// Wrap children in a focusable element so FocusScope can manage focus +const ModalContent = ({ + children, + text, +}: { + children?: React.ReactNode; + text: string; +}) => ( +
+ + {children} +
+); + const renderStackedModals = ({ childOnClose = vi.fn(), parentOnClose = vi.fn(), @@ -41,20 +58,6 @@ const renderStackedModals = ({ , ); -// Wrap children in a focusable element so FocusScope can manage focus -const ModalContent = ({ - children, - text, -}: { - children?: React.ReactNode; - text: string; -}) => ( -
- - {children} -
-); - const OverlayCloseButton = React.forwardRef< HTMLButtonElement, React.ComponentProps<'button'> @@ -336,8 +339,15 @@ describe('GlobalModal', () => { it('supports rendering an alertdialog above an existing modal', () => { renderStackedModals(); - expect(screen.getByRole('dialog', { name: 'Parent modal' })).toBeInTheDocument(); - expect(screen.getByRole('alertdialog', { name: 'Child modal' })).toBeInTheDocument(); + const parentModal = screen.getByRole('dialog', { name: 'Parent modal' }); + const childModal = screen.getByRole('alertdialog', { name: 'Child modal' }); + + expect(parentModal).toBeInTheDocument(); + expect(parentModal).not.toHaveAttribute('aria-modal'); + expect(parentModal).toHaveAttribute('inert'); + expect(childModal).toBeInTheDocument(); + expect(childModal).toHaveAttribute('aria-modal', 'true'); + expect(childModal).not.toHaveAttribute('inert'); }); it('only closes the topmost modal on Escape', () => { From 58c45de3b3e3c1beed4b2571a414ead878e69775 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 3 Jun 2026 15:13:52 +0200 Subject: [PATCH 4/5] test: post merge test adjustments --- .../Modal/__tests__/GlobalModal.test.tsx | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/components/Modal/__tests__/GlobalModal.test.tsx b/src/components/Modal/__tests__/GlobalModal.test.tsx index fbad2babb..dad65a004 100644 --- a/src/components/Modal/__tests__/GlobalModal.test.tsx +++ b/src/components/Modal/__tests__/GlobalModal.test.tsx @@ -57,19 +57,21 @@ const renderStackedModals = ({ } = {}) => render( - - - - - + + + + + + + - - + + , ); From 17ff97c66c681381842148711fcb55dc07c70368 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 3 Jun 2026 15:20:32 +0200 Subject: [PATCH 5/5] test: add further test fixes --- .../Message/__tests__/MessageText.test.tsx | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/components/Message/__tests__/MessageText.test.tsx b/src/components/Message/__tests__/MessageText.test.tsx index 6c9d45942..230999160 100644 --- a/src/components/Message/__tests__/MessageText.test.tsx +++ b/src/components/Message/__tests__/MessageText.test.tsx @@ -9,6 +9,7 @@ import { ChatProvider, ComponentProvider, DialogManagerProvider, + MessageProvider, TranslationProvider, } from '../../../context'; import { @@ -23,6 +24,7 @@ import { mockChannelStateContext, mockChatContext, mockComponentContext, + mockMessageContext, mockTranslationContextValue, } from '../../../mock-builders'; @@ -89,6 +91,8 @@ async function renderMessageText({ const channelCapabilities = { 'send-reaction': true, ...channelCapabilitiesOverrides }; const channelConfig = channel['getConfig'](); const customDateTimeParser = vi.fn(() => ({ format: vi.fn() })); + const renderMessageTextDirectly = + 'customInnerClass' in customProps || 'customWrapperClass' in customProps; return render( @@ -119,9 +123,28 @@ async function renderMessageText({ })} > - - - + {renderMessageTextDirectly ? ( + + + + ) : ( + + + + )} @@ -420,10 +443,8 @@ describe('', () => { const message = generateMessage({ text: 'hello world' }); const { getByText } = await renderMessageText({ customProps: { + customWrapperClass, message, - Message: () => ( - - ), }, }); @@ -437,10 +458,8 @@ describe('', () => { const message = generateMessage({ text: 'hi mate' }); const { getByTestId } = await renderMessageText({ customProps: { + customInnerClass, message, - Message: () => ( - - ), }, });