From 28a0da91e4db3debc6abadbd7c6e59df3cb5ceed Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Fri, 19 Apr 2024 16:07:42 +0200 Subject: [PATCH 1/3] Improve the link preview accessibility and labels. --- .../components/link-control/link-preview.js | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index e5de47f0153a82..962228583f43e5 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -6,7 +6,7 @@ import clsx from 'clsx'; /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; import { Button, ExternalLink, @@ -96,7 +96,8 @@ export default function LinkPreview( { return (
- + { hasUnlinkControl && (
From a0ce5cdeb82a54896c9e614a4629d2e85050a89f Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Mon, 22 Apr 2024 09:45:16 +0200 Subject: [PATCH 2/3] Adjust test. --- .../src/components/link-control/test/index.js | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index e416b9b522a53c..dc8291fc9a1933 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -373,7 +373,7 @@ describe( 'Basic rendering', () => { /> ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); const isPreviewError = linkPreview.classList.contains( 'is-error' ); expect( isPreviewError ).toBe( true ); @@ -834,7 +834,7 @@ describe( 'Manual link entry', () => { render( ); - let linkPreview = screen.getByLabelText( 'Currently selected' ); + let linkPreview = screen.getByLabelText( 'Manage link' ); expect( linkPreview ).toBeInTheDocument(); @@ -868,7 +868,7 @@ describe( 'Manual link entry', () => { // Cancel the editing process. await user.click( cancelButton ); - linkPreview = screen.getByLabelText( 'Currently selected' ); + linkPreview = screen.getByLabelText( 'Manage link' ); expect( linkPreview ).toBeInTheDocument(); @@ -1076,7 +1076,7 @@ describe( 'Default search suggestions', () => { // Click the "Edit/Change" button and check initial suggestions are not // shown. - const currentLinkUI = screen.getByLabelText( 'Currently selected' ); + const currentLinkUI = screen.getByLabelText( 'Manage link' ); const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', { name: 'Edit link', } ); @@ -1230,8 +1230,7 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { // Check for loading indicator. const loadingIndicator = screen.getByText( 'Creating…' ); - const currentLinkLabel = - screen.queryByLabelText( 'Currently selected' ); + const currentLinkLabel = screen.queryByLabelText( 'Manage link' ); expect( currentLinkLabel ).not.toBeInTheDocument(); expect( loadingIndicator ).toBeVisible(); @@ -1242,8 +1241,7 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { // Resolve the `createSuggestion` promise. resolver(); - const currentLink = - await screen.findByLabelText( 'Currently selected' ); + const currentLink = await screen.findByLabelText( 'Manage link' ); expect( currentLink ).toHaveTextContent( entityNameText ); expect( currentLink ).toHaveTextContent( '/?p=123' ); @@ -1291,7 +1289,7 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { await user.click( createButton ); - const currentLink = screen.getByLabelText( 'Currently selected' ); + const currentLink = screen.getByLabelText( 'Manage link' ); expect( currentLink ).toHaveTextContent( 'Some new page to create' ); expect( currentLink ).toHaveTextContent( '/?p=123' ); @@ -1350,7 +1348,7 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { triggerEnter( searchInput ); expect( - await screen.findByLabelText( 'Currently selected' ) + await screen.findByLabelText( 'Manage link' ) ).toHaveTextContent( entityNameText ); } ); @@ -1529,7 +1527,7 @@ describe( 'Selecting links', () => { render( ); - const currentLink = screen.getByLabelText( 'Currently selected' ); + const currentLink = screen.getByLabelText( 'Manage link' ); const currentLinkAnchor = screen.getByRole( 'link', { name: `${ selectedLink.title } (opens in a new tab)`, } ); @@ -1559,7 +1557,7 @@ describe( 'Selecting links', () => { render( ); // Required in order to select the button below. - let currentLinkUI = screen.getByLabelText( 'Currently selected' ); + let currentLinkUI = screen.getByLabelText( 'Manage link' ); const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', { name: 'Edit link', } ); @@ -1570,7 +1568,7 @@ describe( 'Selecting links', () => { const searchInput = screen.getByRole( 'combobox', { name: 'Search or type URL', } ); - currentLinkUI = screen.queryByLabelText( 'Currently selected' ); + currentLinkUI = screen.queryByLabelText( 'Manage link' ); // We should be back to showing the search input. expect( searchInput ).toBeVisible(); @@ -1733,8 +1731,7 @@ describe( 'Selecting links', () => { triggerEnter( searchInput ); // Check that the suggestion selected via is now shown as selected. - const currentLink = - screen.getByLabelText( 'Currently selected' ); + const currentLink = screen.getByLabelText( 'Manage link' ); const currentLinkAnchor = screen.getByRole( 'link', { name: `${ selectedLink.title } (opens in a new tab)`, } ); @@ -2127,7 +2124,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); @@ -2148,7 +2145,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); } ); @@ -2165,7 +2162,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2197,7 +2194,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2221,7 +2218,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2256,7 +2253,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) @@ -2281,7 +2278,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); expect( linkPreview ).toHaveClass( 'is-fetching' ); @@ -2300,7 +2297,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); expect( linkPreview ).toHaveClass( 'is-fetching' ); expect( linkPreview ).not.toHaveClass( 'is-rich' ); @@ -2313,7 +2310,7 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Currently selected' ); + const linkPreview = screen.getByLabelText( 'Manage link' ); expect( linkPreview ).toHaveClass( 'is-fetching' ); From c737c13a5ecd75339c8c24f74fa34fb123a60900 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Fri, 26 Apr 2024 15:55:47 +0200 Subject: [PATCH 3/3] Update test by using ByRole queries. --- .../src/components/link-control/test/index.js | 84 ++++++++++++++----- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index dc8291fc9a1933..bd97fec4ba0073 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -373,7 +373,9 @@ describe( 'Basic rendering', () => { /> ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); const isPreviewError = linkPreview.classList.contains( 'is-error' ); expect( isPreviewError ).toBe( true ); @@ -834,7 +836,9 @@ describe( 'Manual link entry', () => { render( ); - let linkPreview = screen.getByLabelText( 'Manage link' ); + let linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( linkPreview ).toBeInTheDocument(); @@ -868,7 +872,9 @@ describe( 'Manual link entry', () => { // Cancel the editing process. await user.click( cancelButton ); - linkPreview = screen.getByLabelText( 'Manage link' ); + linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( linkPreview ).toBeInTheDocument(); @@ -1076,7 +1082,9 @@ describe( 'Default search suggestions', () => { // Click the "Edit/Change" button and check initial suggestions are not // shown. - const currentLinkUI = screen.getByLabelText( 'Manage link' ); + const currentLinkUI = screen.getByRole( 'group', { + name: 'Manage link', + } ); const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', { name: 'Edit link', } ); @@ -1230,7 +1238,9 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { // Check for loading indicator. const loadingIndicator = screen.getByText( 'Creating…' ); - const currentLinkLabel = screen.queryByLabelText( 'Manage link' ); + const currentLinkLabel = screen.queryByRole( 'group', { + name: 'Manage link', + } ); expect( currentLinkLabel ).not.toBeInTheDocument(); expect( loadingIndicator ).toBeVisible(); @@ -1241,7 +1251,9 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { // Resolve the `createSuggestion` promise. resolver(); - const currentLink = await screen.findByLabelText( 'Manage link' ); + const currentLink = await screen.findByRole( 'group', { + name: 'Manage link', + } ); expect( currentLink ).toHaveTextContent( entityNameText ); expect( currentLink ).toHaveTextContent( '/?p=123' ); @@ -1289,7 +1301,9 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { await user.click( createButton ); - const currentLink = screen.getByLabelText( 'Manage link' ); + const currentLink = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( currentLink ).toHaveTextContent( 'Some new page to create' ); expect( currentLink ).toHaveTextContent( '/?p=123' ); @@ -1348,7 +1362,9 @@ describe( 'Creating Entities (eg: Posts, Pages)', () => { triggerEnter( searchInput ); expect( - await screen.findByLabelText( 'Manage link' ) + await screen.findByRole( 'group', { + name: 'Manage link', + } ) ).toHaveTextContent( entityNameText ); } ); @@ -1527,7 +1543,9 @@ describe( 'Selecting links', () => { render( ); - const currentLink = screen.getByLabelText( 'Manage link' ); + const currentLink = screen.getByRole( 'group', { + name: 'Manage link', + } ); const currentLinkAnchor = screen.getByRole( 'link', { name: `${ selectedLink.title } (opens in a new tab)`, } ); @@ -1557,7 +1575,9 @@ describe( 'Selecting links', () => { render( ); // Required in order to select the button below. - let currentLinkUI = screen.getByLabelText( 'Manage link' ); + let currentLinkUI = screen.getByRole( 'group', { + name: 'Manage link', + } ); const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', { name: 'Edit link', } ); @@ -1568,7 +1588,9 @@ describe( 'Selecting links', () => { const searchInput = screen.getByRole( 'combobox', { name: 'Search or type URL', } ); - currentLinkUI = screen.queryByLabelText( 'Manage link' ); + currentLinkUI = screen.queryByRole( 'group', { + name: 'Manage link', + } ); // We should be back to showing the search input. expect( searchInput ).toBeVisible(); @@ -1731,7 +1753,9 @@ describe( 'Selecting links', () => { triggerEnter( searchInput ); // Check that the suggestion selected via is now shown as selected. - const currentLink = screen.getByLabelText( 'Manage link' ); + const currentLink = screen.getByRole( 'group', { + name: 'Manage link', + } ); const currentLinkAnchor = screen.getByRole( 'link', { name: `${ selectedLink.title } (opens in a new tab)`, } ); @@ -2124,7 +2148,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); @@ -2145,7 +2171,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); } ); @@ -2162,7 +2190,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2194,7 +2224,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2218,7 +2250,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) ); @@ -2253,7 +2287,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); await waitFor( () => expect( linkPreview ).toHaveClass( 'is-rich' ) @@ -2278,7 +2314,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( linkPreview ).toHaveClass( 'is-fetching' ); @@ -2297,7 +2335,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( linkPreview ).toHaveClass( 'is-fetching' ); expect( linkPreview ).not.toHaveClass( 'is-rich' ); @@ -2310,7 +2350,9 @@ describe( 'Rich link previews', () => { render( ); - const linkPreview = screen.getByLabelText( 'Manage link' ); + const linkPreview = screen.getByRole( 'group', { + name: 'Manage link', + } ); expect( linkPreview ).toHaveClass( 'is-fetching' );