Mm 62677 modal focus second part (#30099)
* MM-62312 - modal focus management; revamp quick switch channel modal! * get quick switch test working * configure the generic modal to accept refs to focus within and onhide to the origin element * apply pr feedback, get modal element get autofocus, use id instead of ref * update more direct channels modal to use generic modal * fix unit tests and snapshots * fix unit tests * fix modal margin top to fit in smaller screens * fix e2e test * remove unnecesary onexited extra call * fix e2e tests * set correct label * fix snapshots * create helper function for sending custom focus event * migrate quick switch modal to use new approach to focus * migrate more direct channels modal to new approach * fix snapshots * fix types * fix modal closing behavior * fix snapshots * fix cypress tests * remove only * MM-62677 - migrate modals, invite modal work * user settings modal * fix snapshots * finish user settings migration * migrate confirm modal to use generic modal * notification preferences migration * implement focus back to trigger to channel notifications modal * fix test snapshots * initial self code review * fix CI errors, translation and some types * add modal location param and adjust test * fix cypress test text * fix cypress test text * fix e2e test for invitation modal * fix e2e test selector * adjust modal height * temp * fix e2e tests * fix snapshot * fix e2e tests * fix snapshots * fix snapshots * fix snapshots * fix e2e tests * update snapshots * fix snapshots * fix linter * Implement PR feedback * fix e2e tests * adjust styling for channel notifications modal * more fixes to e2e tests --------- Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Pablo Vélez
@@ -18,6 +18,7 @@ exports[`GenericModal should match snapshot for base case 1`] = `
|
||||
/>
|
||||
<div
|
||||
aria-labelledby="genericModalLabel"
|
||||
aria-modal="true"
|
||||
class="fade in modal"
|
||||
id="genericModal"
|
||||
role="dialog"
|
||||
@@ -25,14 +26,14 @@ exports[`GenericModal should match snapshot for base case 1`] = `
|
||||
tabindex="-1"
|
||||
>
|
||||
<div
|
||||
class="a11y__modal GenericModal modal-dialog"
|
||||
class="GenericModal__location--center a11y__modal GenericModal modal-dialog"
|
||||
>
|
||||
<div
|
||||
class="modal-content"
|
||||
role="document"
|
||||
>
|
||||
<div
|
||||
class="GenericModal__wrapper-enter-key-press-catcher"
|
||||
class="GenericModal__wrapper GenericModal__wrapper-enter-key-press-catcher"
|
||||
tabindex="0"
|
||||
>
|
||||
<div
|
||||
|
||||
@@ -1,9 +1,17 @@
|
||||
.console__body .modal .GenericModal,
|
||||
.app__body .modal .GenericModal {
|
||||
&.modal-dialog {
|
||||
&__location--top {
|
||||
margin-top: 5vh;
|
||||
}
|
||||
|
||||
&__location--center {
|
||||
margin-top: calc(50vh - 240px);
|
||||
}
|
||||
|
||||
&__location--bottom {
|
||||
margin-top: calc(50vh + 240px);
|
||||
}
|
||||
|
||||
&.modal-overflow {
|
||||
.modal-body {
|
||||
overflow: visible;
|
||||
@@ -131,7 +139,7 @@
|
||||
}
|
||||
|
||||
@media screen and (max-width: 640px) {
|
||||
margin: 0;
|
||||
margin: 0 !important;
|
||||
|
||||
.modal-header {
|
||||
box-shadow: var(--elevation-2);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||
// See LICENSE.txt for license information.
|
||||
|
||||
import {render, screen} from '@testing-library/react';
|
||||
import {render, screen, waitFor} from '@testing-library/react';
|
||||
import React from 'react';
|
||||
|
||||
import {GenericModal} from './generic_modal';
|
||||
@@ -37,4 +37,108 @@ describe('GenericModal', () => {
|
||||
expect(screen.getByText('Confirm')).toBeInTheDocument();
|
||||
expect(screen.getByText('Cancel')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('calls onExited when modal exits', async () => {
|
||||
const onExitedMock = jest.fn();
|
||||
const props = {
|
||||
...baseProps,
|
||||
onExited: onExitedMock,
|
||||
};
|
||||
render(
|
||||
wrapIntl(<GenericModal {...props}/>),
|
||||
);
|
||||
|
||||
// Find and click the close button to trigger modal exit
|
||||
const closeButton = screen.getByLabelText('Close');
|
||||
closeButton.click();
|
||||
|
||||
// Wait for onExited to be called
|
||||
await waitFor(() => {
|
||||
expect(onExitedMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
test('does not throw if onExited is undefined', async () => {
|
||||
// Create props without onExited
|
||||
const {onExited, ...propsWithoutOnExited} = baseProps; // eslint-disable-line @typescript-eslint/no-unused-vars
|
||||
const props = {
|
||||
...propsWithoutOnExited,
|
||||
onHide: jest.fn(), // Ensure onHide is provided since it's mandatory
|
||||
};
|
||||
|
||||
// This should not throw
|
||||
render(
|
||||
wrapIntl(<GenericModal {...props}/>),
|
||||
);
|
||||
|
||||
// Find and click the close button to trigger modal exit
|
||||
const closeButton = screen.getByLabelText('Close');
|
||||
|
||||
// This should not throw
|
||||
expect(() => {
|
||||
closeButton.click();
|
||||
}).not.toThrow();
|
||||
});
|
||||
|
||||
test('calls onEntered when modal enters', async () => {
|
||||
const onEnteredMock = jest.fn();
|
||||
const props = {
|
||||
...baseProps,
|
||||
onEntered: onEnteredMock,
|
||||
show: false, // Start with modal hidden
|
||||
};
|
||||
|
||||
const {rerender} = render(
|
||||
wrapIntl(<GenericModal {...props}/>),
|
||||
);
|
||||
|
||||
// Show the modal
|
||||
rerender(
|
||||
wrapIntl(<GenericModal {...props} show={true}/>),
|
||||
);
|
||||
|
||||
// Wait for onEntered to be called
|
||||
await waitFor(() => {
|
||||
expect(onEnteredMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
test('does not throw if onEntered is undefined', async () => {
|
||||
// Create props without onEntered
|
||||
const props = {
|
||||
...baseProps,
|
||||
show: false, // Start with modal hidden
|
||||
};
|
||||
|
||||
const {rerender} = render(
|
||||
wrapIntl(<GenericModal {...props}/>),
|
||||
);
|
||||
|
||||
// This should not throw
|
||||
expect(() => {
|
||||
rerender(
|
||||
wrapIntl(<GenericModal {...props} show={true}/>),
|
||||
);
|
||||
}).not.toThrow();
|
||||
});
|
||||
|
||||
test('calls onHide when modal is closed', async () => {
|
||||
const onHideMock = jest.fn();
|
||||
const props = {
|
||||
...baseProps,
|
||||
onHide: onHideMock,
|
||||
};
|
||||
render(
|
||||
wrapIntl(<GenericModal {...props}/>),
|
||||
);
|
||||
|
||||
// Find and click the close button to trigger modal exit
|
||||
const closeButton = screen.getByLabelText('Close');
|
||||
closeButton.click();
|
||||
|
||||
// Wait for onHide to be called
|
||||
await waitFor(() => {
|
||||
expect(onHideMock).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,9 +8,11 @@ import {FormattedMessage} from 'react-intl';
|
||||
|
||||
import './generic_modal.scss';
|
||||
|
||||
export type ModalLocation = 'top' | 'center' | 'bottom';
|
||||
|
||||
export type Props = {
|
||||
className?: string;
|
||||
onExited: () => void;
|
||||
onExited?: () => void;
|
||||
onEntered?: () => void;
|
||||
onHide?: () => void;
|
||||
modalHeaderText?: React.ReactNode;
|
||||
@@ -32,6 +34,7 @@ export type Props = {
|
||||
enforceFocus?: boolean;
|
||||
container?: React.ReactNode | React.ReactNodeArray;
|
||||
ariaLabel?: string;
|
||||
ariaLabelledby?: string;
|
||||
errorText?: string | React.ReactNode;
|
||||
compassDesign?: boolean;
|
||||
backdrop?: boolean | 'static';
|
||||
@@ -48,6 +51,22 @@ export type Props = {
|
||||
footerDivider?: boolean;
|
||||
appendedContent?: React.ReactNode;
|
||||
headerButton?: React.ReactNode;
|
||||
showCloseButton?: boolean;
|
||||
showHeader?: boolean;
|
||||
|
||||
/*
|
||||
* Controls the vertical location of the modal.
|
||||
* 'top' => margin-top: 5vh
|
||||
* 'center' => margin-top: calc(50vh - 240px)
|
||||
* 'bottom' => margin-top: calc(50vh + 240px) (example calculation)
|
||||
*/
|
||||
modalLocation?: ModalLocation;
|
||||
|
||||
/**
|
||||
* Optionally set a test ID for the container, so that the modal can be easily referenced
|
||||
* in tests (Cypress, Playwright, etc.)
|
||||
*/
|
||||
dataTestId?: string;
|
||||
};
|
||||
|
||||
type State = {
|
||||
@@ -63,6 +82,9 @@ export class GenericModal extends React.PureComponent<Props, State> {
|
||||
enforceFocus: true,
|
||||
keyboardEscape: true,
|
||||
bodyPadding: true,
|
||||
showCloseButton: true,
|
||||
showHeader: true,
|
||||
modalLocation: 'center',
|
||||
};
|
||||
|
||||
constructor(props: Props) {
|
||||
@@ -184,13 +206,33 @@ export class GenericModal extends React.PureComponent<Props, State> {
|
||||
</div>
|
||||
);
|
||||
|
||||
const locationClassMapping: Record<Required<Props>['modalLocation'], string> = {
|
||||
top: 'GenericModal__location--top',
|
||||
center: 'GenericModal__location--center',
|
||||
bottom: 'GenericModal__location--bottom',
|
||||
};
|
||||
|
||||
const modalLocationClass = locationClassMapping[this.props.modalLocation ?? 'center'];
|
||||
|
||||
// Accessibility labeling strategy:
|
||||
// 1. We always set aria-labelledby to ensure the modal has a proper label
|
||||
// - First try to use the provided ariaLabeledBy prop
|
||||
// - Fall back to 'genericModalLabel' which references the modal title
|
||||
// 2. We also support aria-label as a secondary option
|
||||
// - This will only be used by screen readers if the element referenced by aria-labelledby doesn't exist
|
||||
// - This provides a fallback for accessibility in case the referenced element is missing
|
||||
// Note: When both aria-labelledby and aria-label are present, aria-labelledby takes precedence
|
||||
const ariaLabelledby = this.props.ariaLabelledby || 'genericModalLabel';
|
||||
|
||||
return (
|
||||
<Modal
|
||||
id={this.props.id}
|
||||
role='none'
|
||||
aria-label={this.props.ariaLabel}
|
||||
aria-labelledby={this.props.ariaLabel ? undefined : 'genericModalLabel'}
|
||||
aria-labelledby={ariaLabelledby}
|
||||
aria-modal='true'
|
||||
dialogClassName={classNames(
|
||||
modalLocationClass,
|
||||
'a11y__modal GenericModal',
|
||||
{
|
||||
GenericModal__compassDesign: this.props.compassDesign,
|
||||
@@ -208,13 +250,14 @@ export class GenericModal extends React.PureComponent<Props, State> {
|
||||
container={this.props.container}
|
||||
keyboard={this.props.keyboardEscape}
|
||||
onEntered={this.props.onEntered}
|
||||
data-testid={this.props.dataTestId}
|
||||
>
|
||||
<div
|
||||
onKeyDown={this.onEnterKeyDown}
|
||||
tabIndex={this.props.tabIndex || 0}
|
||||
className='GenericModal__wrapper-enter-key-press-catcher'
|
||||
className='GenericModal__wrapper GenericModal__wrapper-enter-key-press-catcher'
|
||||
>
|
||||
<Modal.Header closeButton={true}>
|
||||
{this.props.showHeader && <Modal.Header closeButton={this.props.showCloseButton}>
|
||||
<div
|
||||
className='GenericModal__header__text_container'
|
||||
>
|
||||
@@ -236,7 +279,7 @@ export class GenericModal extends React.PureComponent<Props, State> {
|
||||
</div>
|
||||
}
|
||||
</div>
|
||||
</Modal.Header>
|
||||
</Modal.Header>}
|
||||
<Modal.Body className={classNames({divider: this.props.bodyDivider, 'overflow-visible': this.props.bodyOverflowVisible})}>
|
||||
{this.props.compassDesign ? (
|
||||
this.props.errorText && (
|
||||
|
||||
Ссылка в новой задаче
Block a user