Skip to content
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.

Sign up for GitHub

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

Modal: make everything outside inert #53829

Draft
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Draft

Modal: make everything outside inert #53829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca91955
Make content outside of `Modal` inert
stokesman Jul 3, 2023
821be7a
Update unit tests for Modal’s aria helper
stokesman Jul 3, 2023
7601474
Fix focus return when dismissed
stokesman Jan 19, 2024
ca14a75
Add hack for Chromium bug
stokesman Aug 22, 2023
5075de3
Update unit test
stokesman Jan 19, 2024
c7fbbc5
Revert "Add hack for Chromium bug"
stokesman Feb 19, 2025
5d9ac19
Update font library e2e tests
stokesman Feb 19, 2025
80ca85d
Update Block template registration e2e test
stokesman Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/components/src/modal/aria-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function modalize( modalElement?: HTMLDivElement ) {
}

if ( elementShouldBeHidden( element ) ) {
element.setAttribute( 'aria-hidden', 'true' );
element.setAttribute( 'inert', '' );
hiddenElements.push( element );
}
}
Expand All @@ -49,6 +49,7 @@ export function elementShouldBeHidden( element: Element ) {
element.tagName === 'SCRIPT' ||
element.hasAttribute( 'hidden' ) ||
element.hasAttribute( 'aria-hidden' ) ||
element.hasAttribute( 'inert' ) ||
element.hasAttribute( 'aria-live' ) ||
( role && LIVE_REGION_ARIA_ROLES.has( role ) )
);
Expand All @@ -64,6 +65,6 @@ export function unmodalize() {
}

for ( const element of hiddenElements ) {
element.removeAttribute( 'aria-hidden' );
element.removeAttribute( 'inert' );
}
}
2 changes: 1 addition & 1 deletion packages/components/src/modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function UnforwardedModal(
}, [ contentRef ] );

// Accessibly isolates/unisolates the modal.
useEffect( () => {
useLayoutEffect( () => {
ariaHelper.modalize( ref.current );
return () => ariaHelper.unmodalize();
}, [] );
Expand Down
11 changes: 2 additions & 9 deletions packages/components/src/modal/test/aria-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,9 @@ describe( 'aria-helper', () => {
expect( elementShouldBeHidden( element ) ).toBe( false );
} );

it( 'should return false when an element has the aria-hidden attribute with value "true"', () => {
it( 'should return false when an element has the inert attribute', () => {
const element = document.createElement( 'div' );
element.setAttribute( 'aria-hidden', 'true' );

expect( elementShouldBeHidden( element ) ).toBe( false );
} );

it( 'should return false when an element has the aria-hidden attribute with value "false"', () => {
const element = document.createElement( 'div' );
element.setAttribute( 'aria-hidden', 'false' );
element.setAttribute( 'inert', '' );

expect( elementShouldBeHidden( element ) ).toBe( false );
} );
Expand Down
10 changes: 5 additions & 5 deletions packages/components/src/modal/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,24 +234,24 @@ describe( 'Modal', () => {

// Opens outer modal > hides container.
await user.click( screen.getByRole( 'button', { name: 'Start' } ) );
expect( container ).toHaveAttribute( 'aria-hidden', 'true' );
expect( container ).toHaveAttribute( 'inert' );

// Disable reason: No semantic query can reach the overlay.
// eslint-disable-next-line testing-library/no-node-access
const outer = screen.getByRole( 'dialog' ).parentElement!;

// Opens inner modal > hides outer modal.
await user.click( screen.getByRole( 'button', { name: 'Nest' } ) );
expect( outer ).toHaveAttribute( 'aria-hidden', 'true' );
expect( outer ).toHaveAttribute( 'inert' );

// Closes inner modal > Unhides outer modal and container stays hidden.
await user.keyboard( '[Escape]' );
expect( outer ).not.toHaveAttribute( 'aria-hidden' );
expect( container ).toHaveAttribute( 'aria-hidden', 'true' );
expect( outer ).not.toHaveAttribute( 'inert' );
expect( container ).toHaveAttribute( 'inert' );

// Closes outer modal > Unhides container.
await user.keyboard( '[Escape]' );
expect( container ).not.toHaveAttribute( 'aria-hidden' );
expect( container ).not.toHaveAttribute( 'inert' );
} );

it( 'should render `headerActions` React nodes', async () => {
Expand Down
36 changes: 21 additions & 15 deletions test/e2e/specs/site-editor/font-library.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ test.describe( 'Font Library', () => {
name: 'Manage fonts',
} )
.click();
await expect( page.getByRole( 'dialog' ) ).toBeVisible();
await expect(
page.getByRole( 'heading', { name: 'Fonts', exact: true } )
page
.getByRole( 'dialog' )
.getByRole( 'heading', { name: 'Fonts', exact: true } )
).toBeVisible();
} );

Expand All @@ -106,12 +107,13 @@ test.describe( 'Font Library', () => {
name: 'Manage fonts',
} )
.click();
await page.getByRole( 'button', { name: 'System Font' } ).click();
const dialog = page.getByRole( 'dialog' );
await dialog.getByRole( 'button', { name: 'System Font' } ).click();
await expect(
page.getByRole( 'heading', { name: 'System Font' } )
dialog.getByRole( 'heading', { name: 'System Font' } )
).toBeVisible();
await expect(
page.getByRole( 'checkbox', { name: 'System Font Normal' } )
dialog.getByRole( 'checkbox', { name: 'System Font Normal' } )
).toBeVisible();
} );
} );
Expand Down Expand Up @@ -174,15 +176,16 @@ test.describe( 'Font Library', () => {
.getByText( 'Fonts were installed successfully.' )
).toBeVisible();
await page.getByRole( 'tab', { name: 'Library' } ).click();
const dialog = page.getByRole( 'dialog' );
// Provides coverage for https://github.com/WordPress/gutenberg/issues/60040.
await page.getByRole( 'button', { name: 'Exo 2' } ).click();
await dialog.getByRole( 'button', { name: 'Exo 2' } ).click();
await expect( page.getByLabel( 'Exo 2 Normal' ) ).toBeVisible();
await expect(
page.getByLabel( 'Exo 2 Semi-bold Italic' )
).toBeVisible();

// Check CSS preset was created.
await page.getByRole( 'button', { name: 'Close' } ).click();
await dialog.getByRole( 'button', { name: 'Close' } ).click();
await page
.getByRole( 'button', { name: 'Headings', exact: true } )
.click();
Expand All @@ -200,9 +203,13 @@ test.describe( 'Font Library', () => {
} )
.click();

await page.getByRole( 'button', { name: 'Exo 2' } ).click();
await page.getByRole( 'button', { name: 'Delete' } ).click();
await page.getByRole( 'button', { name: 'Delete' } ).click();
await dialog.getByRole( 'button', { name: 'Exo 2' } ).click();
await dialog.getByRole( 'button', { name: 'Delete' } ).click();
await page
.getByRole( 'dialog' )
.last()
.getByRole( 'button', { name: 'Delete' } )
.click();
await expect(
page
.getByLabel( 'Library' )
Expand Down Expand Up @@ -248,9 +255,9 @@ test.describe( 'Font Library', () => {
.getByRole( 'button', { name: 'Jost 2 variants' } )
.click();

await expect( page.getByRole( 'dialog' ) ).toBeVisible();
const dialog = page.getByRole( 'dialog' );
await expect(
page.getByRole( 'heading', { name: 'Fonts' } )
dialog.getByRole( 'heading', { name: 'Fonts' } )
).toBeVisible();

// Check correct font is displayed in Font Library
Expand All @@ -259,7 +266,7 @@ test.describe( 'Font Library', () => {
).toBeVisible();

// Close the Font Library dialog
await page.getByRole( 'button', { name: 'Close' } ).click();
await dialog.getByRole( 'button', { name: 'Close' } ).click();

// Click "Back"
await page.getByRole( 'button', { name: 'Back' } ).click();
Expand All @@ -280,9 +287,8 @@ test.describe( 'Font Library', () => {
.getByRole( 'button', { name: 'Cardo 3 variants' } )
.click();

await expect( page.getByRole( 'dialog' ) ).toBeVisible();
await expect(
page.getByRole( 'heading', { name: 'Fonts' } )
dialog.getByRole( 'heading', { name: 'Fonts' } )
).toBeVisible();

// Check correct font is displayed in Font Library
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/specs/site-editor/template-registration.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ test.describe( 'Block template registration', () => {
const searchResults = page.getByLabel( 'Actions' );
await searchResults.first().click();
await page.getByRole( 'menuitem', { name: 'Reset' } ).click();
await page.getByRole( 'button', { name: 'Reset' } ).click();
await page
.getByRole( 'dialog' )
.getByRole( 'button', { name: 'Reset' } )
.click();

await expect( resetNotice ).toBeVisible();
await expect( savedButton ).toBeVisible();
Expand Down
Loading