diff --git a/apps/meteor/client/views/room/MessageList/MessageList.spec.tsx b/apps/meteor/client/views/room/MessageList/MessageList.spec.tsx index 474aa193610f1..e230094c9fd08 100644 --- a/apps/meteor/client/views/room/MessageList/MessageList.spec.tsx +++ b/apps/meteor/client/views/room/MessageList/MessageList.spec.tsx @@ -1,8 +1,10 @@ import type { IMessage, IRoom, IUser } from '@rocket.chat/core-typings'; import { mockAppRoot } from '@rocket.chat/mock-providers'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { axe } from 'jest-axe'; import type { ReactNode } from 'react'; +import type { MessageListProps } from './MessageList'; import { MessageList } from './MessageList'; import { useMessages } from './hooks/useMessages'; import { RoomManager } from '../../../lib/RoomManager'; @@ -18,19 +20,27 @@ const mockVirtualizerHandle = { }; jest.mock('virtua', () => { - const { forwardRef, useImperativeHandle } = jest.requireActual('react'); + const { Children, forwardRef, useImperativeHandle } = jest.requireActual('react'); return { + // `virtua` renders a plain container and wraps every child in a `div` of its own, so the list markup + // under test cannot rely on `ul`/`li` semantics. VList: forwardRef( ( - { children, onScroll, shift: _shift, ...props }: { children: ReactNode; onScroll?: (offset: number) => void; shift?: boolean }, + { + children, + onScroll, + shift: _shift, + keepMounted: _keepMounted, + ...props + }: { children: ReactNode; onScroll?: (offset: number) => void; shift?: boolean; keepMounted?: number[] }, ref: any, ) => { useImperativeHandle(ref, () => mockVirtualizerHandle); return ( - +
onScroll?.(mockVirtualizerHandle.scrollOffset)} {...props}> + {Children.map(children, (child) => (child ?
{child}
: child))} +
); }, ), @@ -70,7 +80,11 @@ jest.mock('../contexts/ChatContext', () => ({ })); jest.mock('./MessageListItem', () => ({ - MessageListItem: ({ message }: { message: IMessage }) =>
  • {message.msg}
  • , + MessageListItem: ({ message }: { message: IMessage }) => ( +
    + {message.msg} +
    + ), })); jest.mock('./providers/MessageListProvider', () => ({ children }: { children: ReactNode }) => <>{children}); @@ -211,3 +225,42 @@ describe('MessageList scroll position', () => { }); }); }); + +describe('MessageList accessibility', () => { + let root: ReturnType; + + beforeEach(() => { + jest.clearAllMocks(); + (useMessages as jest.Mock).mockReturnValue([createMessage('message-1'), createMessage('message-2')]); + (useFirstUnreadMessageId as jest.Mock).mockReturnValue(undefined); + (RoomManager.getStore as jest.Mock).mockReturnValue({ scroll: undefined, atBottom: false, update: jest.fn() }); + root = mockAppRoot().withSetting('Message_GroupingPeriod', 300).withUserPreference('displayAvatars', true); + }); + + it('should render a labelled list exposing every message as a list item', () => { + render(, { wrapper: root.build() }); + + const list = screen.getByRole('list'); + + expect(list).toHaveAccessibleName(); + // the two messages plus the foreword + expect(within(list).getAllByRole('listitem')).toHaveLength(3); + }); + + const states: [string, Partial][] = [ + ['default', {}], + ['loading previous messages', { hasMorePreviousMessages: true, isLoadingMoreMessages: true }], + ['loading next messages', { hasMoreNextMessages: true, isLoadingMoreMessages: true }], + [ + 'showing the retention policy warning', + { retentionPolicy: { enabled: true, isActive: true, filesOnly: false, excludePinned: false, ignoreThreads: false, maxAge: 30 } }, + ], + ['without preview permission', { canPreview: false }], + ]; + + it.each(states)('should have no accessibility violations when %s', async (_state, props) => { + const { container } = render(, { wrapper: root.build() }); + + expect(await axe(container)).toHaveNoViolations(); + }); +}); diff --git a/apps/meteor/client/views/room/MessageList/MessageList.tsx b/apps/meteor/client/views/room/MessageList/MessageList.tsx index 8c62588a97525..f77741e4592c6 100644 --- a/apps/meteor/client/views/room/MessageList/MessageList.tsx +++ b/apps/meteor/client/views/room/MessageList/MessageList.tsx @@ -282,12 +282,14 @@ export const MessageList = function MessageList({ {canPreview ? ( <> {hasMorePreviousMessages ? ( -
  • {isLoadingMoreMessages ? : null}
  • +
    + {isLoadingMoreMessages ? : null} +
    ) : ( -
  • +
    {retentionPolicy?.isActive ? : null} -
  • + )} ) : null} @@ -316,7 +318,11 @@ export const MessageList = function MessageList({ ); })} - {hasMoreNextMessages ?
  • {isLoadingMoreMessages ? : null}
  • : null} + {hasMoreNextMessages ? ( +
    + {isLoadingMoreMessages ? : null} +
    + ) : null} diff --git a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.spec.tsx b/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.spec.tsx index 87fd37fa2865c..f85038c31348e 100644 --- a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.spec.tsx +++ b/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.spec.tsx @@ -1,7 +1,8 @@ import type { IMessage, IThreadMainMessage, IThreadMessage } from '@rocket.chat/core-typings'; import { mockAppRoot } from '@rocket.chat/mock-providers'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { axe } from 'jest-axe'; import type { HTMLAttributes, ReactNode } from 'react'; import { forwardRef } from 'react'; @@ -19,9 +20,11 @@ const mockVirtualizerHandle = { }; jest.mock('virtua', () => { - const { forwardRef, useImperativeHandle } = jest.requireActual('react'); + const { Children, forwardRef, useImperativeHandle } = jest.requireActual('react'); return { + // `virtua` renders a plain container and wraps every child in a `div` of its own, so the list markup + // under test cannot rely on `ul`/`li` semantics. VList: forwardRef( ( { @@ -40,9 +43,9 @@ jest.mock('virtua', () => { ) => { useImperativeHandle(ref, () => mockVirtualizerHandle); return ( -
      onScroll?.(mockVirtualizerHandle.scrollOffset)} {...props}> - {children} -
    +
    onScroll?.(mockVirtualizerHandle.scrollOffset)} {...props}> + {Children.map(children, (child) => (child ?
    {child}
    : child))} +
    ); }, ), @@ -107,7 +110,7 @@ jest.mock('../../../../../lib/utils/setMessageJumpQueryStringParameter', () => ( })); jest.mock('./ThreadMessageItem', () => ({ - ThreadMessageItem: ({ message }: { message: IMessage }) =>
  • {message._id}
  • , + ThreadMessageItem: ({ message }: { message: IMessage }) =>
    {message._id}
    , })); jest.mock('../../../BubbleDate', () => ({ @@ -175,3 +178,65 @@ describe('ThreadMessageList', () => { expect(fetchPreviousPage).toHaveBeenCalledTimes(1); }); }); + +describe('ThreadMessageList accessibility', () => { + const mainMessage = createFakeMessage({ + _id: 'thread-id', + rid: room._id, + msg: 'main message', + tcount: 1, + u: { + _id: 'user-id', + username: 'user', + name: 'User', + }, + }); + + const mockThreadMessagesQuery = (overrides: Record = {}) => { + (useThreadMessagesQuery as jest.Mock).mockReturnValue({ + data: { messages: [createThreadMessage(1), createThreadMessage(2)] }, + isLoading: false, + fetchNextPage: jest.fn(), + hasNextPage: false, + isFetchingNextPage: false, + fetchPreviousPage: jest.fn(), + hasPreviousPage: false, + isFetchingPreviousPage: false, + loadMessageAround: jest.fn(), + ...overrides, + }); + }; + + const renderThreadMessageList = () => + render(, { + wrapper: mockAppRoot().withJohnDoe().withSetting('Message_GroupingPeriod', 300).withUserPreference('displayAvatars', true).build(), + }); + + it('should render a labelled list exposing every message as a list item', () => { + mockThreadMessagesQuery(); + + renderThreadMessageList(); + + const list = screen.getByRole('list'); + + expect(list).toHaveAccessibleName(); + // the two replies plus the main message + expect(within(list).getAllByRole('listitem')).toHaveLength(3); + }); + + const states: [string, Record][] = [ + ['default', {}], + ['loading', { isLoading: true, data: undefined }], + ['loading previous messages', { hasPreviousPage: true, isFetchingPreviousPage: true }], + ['loading next messages', { hasNextPage: true, isFetchingNextPage: true }], + ['able to load next messages', { hasNextPage: true }], + ]; + + it.each(states)('should have no accessibility violations when %s', async (_state, overrides) => { + mockThreadMessagesQuery(overrides); + + const { container } = renderThreadMessageList(); + + expect(await axe(container)).toHaveNoViolations(); + }); +}); diff --git a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx b/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx index 6274221f0e7e1..c6ccee4a81d01 100644 --- a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx +++ b/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx @@ -408,12 +408,14 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot }} > {loading ? ( -
  • +
    -
  • + ) : null} {!loading && hasPreviousPage ? ( -
  • {isFetchingPreviousPage ? : null}
  • +
    + {isFetchingPreviousPage ? : null} +
    ) : null} {!loading && items.map((message, index, { [index - 1]: previous }) => { @@ -438,9 +440,9 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot ); })} {!loading && hasNextPage ? ( -
  • +
    {isFetchingNextPage ? : } -
  • + ) : null}