diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index cda7986a9f8c44..5f95d64adde6d9 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -292,8 +292,8 @@ function LinkControl( { ) } suggestion={ suggestion } onClick={ () => { - stopEditing(); onChange( { ...value, ...suggestion } ); + stopEditing(); } } isSelected={ index === selectedSuggestion } isURL={ manualLinkEntryTypes.includes( @@ -318,8 +318,8 @@ function LinkControl( { value={ inputValue } onChange={ onInputChange } onSelect={ ( suggestion ) => { - stopEditing(); onChange( { ...value, ...suggestion } ); + stopEditing(); } } renderSuggestions={ renderSearchResults } fetchSuggestions={ getSearchHandler } 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 44424efe334295..65d7729c973dc4 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -7,7 +7,7 @@ import { first, last, nth } from 'lodash'; /** * WordPress dependencies */ -import { useState } from '@wordpress/element'; +import { useState, useRef } from '@wordpress/element'; import { UP, DOWN, ENTER } from '@wordpress/keycodes'; /** * Internal dependencies @@ -766,6 +766,56 @@ describe( 'Selecting links', () => { } ); } ); + + it( 'does not forcefully regain focus if onChange handler had shifted it', () => { + // Regression: Previously, there had been issues where if `onChange` + // would programmatically shift focus, LinkControl would try to force it + // back, based on its internal logic to determine whether it had focus + // when finishing an edit occuring _before_ `onChange` having been run. + // + // See: https://github.com/WordPress/gutenberg/pull/19462 + + const LinkControlConsumer = () => { + const focusTarget = useRef(); + + return ( + <> +
+ focusTarget.current.focus() } + /> + + ); + }; + + act( () => { + render( , container ); + } ); + + // Change value. + const form = container.querySelector( 'form' ); + const searchInput = container.querySelector( + 'input[aria-label="URL"]' + ); + + // Simulate searching for a term + act( () => { + Simulate.change( searchInput, { + target: { value: 'https://example.com' }, + } ); + } ); + act( () => { + Simulate.keyDown( searchInput, { keyCode: ENTER } ); + } ); + act( () => { + Simulate.submit( form ); + } ); + + const isExpectedFocusTarget = document.activeElement.hasAttribute( + 'data-expected' + ); + expect( isExpectedFocusTarget ).toBe( true ); + } ); } ); describe( 'Addition Settings UI', () => { diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index c6d8d12263e75d..0f21d248a86500 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -7,7 +7,6 @@ import { getEditedPostContent, createNewPost, pressKeyWithModifier, - insertBlock, } from '@wordpress/e2e-test-utils'; /** @@ -44,8 +43,8 @@ describe( 'Links', () => { // Type a URL await page.keyboard.type( 'https://wordpress.org/gutenberg' ); - // Click on the Apply button - await page.click( 'button[aria-label="Apply"]' ); + // Submit the link + await page.keyboard.press( 'Enter' ); // The link should have been inserted expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -80,9 +79,6 @@ describe( 'Links', () => { await clickBlockAppender(); await page.keyboard.type( 'This is Gutenberg: ' ); - // Press escape to show the block toolbar - await page.keyboard.press( 'Escape' ); - // Press Cmd+K to insert a link await pressKeyWithModifier( 'primary', 'K' ); @@ -157,15 +153,16 @@ describe( 'Links', () => { // Type a URL await page.keyboard.type( 'https://wordpress.org/gutenberg' ); - // Click on the Apply button - await page.click( 'button[aria-label="Apply"]' ); + // Click on the Submit button + await page.keyboard.press( 'Enter' ); }; it( 'can be edited', async () => { await createAndReselectLink(); // Click on the Edit button - await page.click( 'button[aria-label="Edit"]' ); + const [ editButton ] = await page.$x( '//button[text()="Edit"]' ); + await editButton.click(); // Wait for the URL field to auto-focus await waitForAutoFocus(); @@ -173,8 +170,8 @@ describe( 'Links', () => { // Change the URL await page.keyboard.type( '/handbook' ); - // Click on the Apply button - await page.click( 'button[aria-label="Apply"]' ); + // Submit the link + await page.keyboard.press( 'Enter' ); // The link should have been updated expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -211,12 +208,16 @@ describe( 'Links', () => { // Typing "left" should not close the dialog await page.keyboard.press( 'ArrowLeft' ); - let popover = await page.$( '.block-editor-url-popover' ); + let popover = await page.$( + '.components-popover__content .block-editor-link-control' + ); expect( popover ).not.toBeNull(); // Escape should close the dialog still. await page.keyboard.press( 'Escape' ); - popover = await page.$( '.block-editor-url-popover' ); + popover = await page.$( + '.components-popover__content .block-editor-link-control' + ); expect( popover ).toBeNull(); } ); @@ -230,12 +231,16 @@ describe( 'Links', () => { // Typing "left" should not close the dialog await page.keyboard.press( 'ArrowLeft' ); - let popover = await page.$( '.block-editor-url-popover' ); + let popover = await page.$( + '.components-popover__content .block-editor-link-control' + ); expect( popover ).not.toBeNull(); // Escape should close the dialog still. await page.keyboard.press( 'Escape' ); - popover = await page.$( '.block-editor-url-popover' ); + popover = await page.$( + '.components-popover__content .block-editor-link-control' + ); expect( popover ).toBeNull(); } ); @@ -247,10 +252,11 @@ describe( 'Links', () => { // Move the mouse to show the block toolbar await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); - await page.click( 'button[aria-label="Edit"]' ); + const [ editButton ] = await page.$x( '//button[text()="Edit"]' ); + await editButton.click(); await waitForAutoFocus(); await page.keyboard.type( '/handbook' ); - await page.click( 'button[aria-label="Apply"]' ); + await page.keyboard.press( 'Enter' ); expect( await getEditedPostContent() ).toMatchSnapshot(); } ); @@ -295,34 +301,54 @@ describe( 'Links', () => { // Wait for the URL field to auto-focus await waitForAutoFocus(); - expect( await page.$( '.block-editor-url-popover' ) ).not.toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).not.toBeNull(); // Trigger the autocomplete suggestion list and select the first suggestion. await page.keyboard.type( titleText ); - await page.waitForSelector( '.block-editor-url-input__suggestion' ); + await page.waitForSelector( '.block-editor-link-control__search-item' ); await page.keyboard.press( 'ArrowDown' ); // Expect the the escape key to dismiss the popover when the autocomplete suggestion list is open. await page.keyboard.press( 'Escape' ); - expect( await page.$( '.block-editor-url-popover' ) ).toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).toBeNull(); // Press Cmd+K to insert a link await pressKeyWithModifier( 'primary', 'K' ); // Wait for the URL field to auto-focus await waitForAutoFocus(); - expect( await page.$( '.block-editor-url-popover' ) ).not.toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).not.toBeNull(); // Expect the the escape key to dismiss the popover normally. await page.keyboard.press( 'Escape' ); - expect( await page.$( '.block-editor-url-popover' ) ).toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).toBeNull(); // Press Cmd+K to insert a link await pressKeyWithModifier( 'primary', 'K' ); // Wait for the URL field to auto-focus await waitForAutoFocus(); - expect( await page.$( '.block-editor-url-popover' ) ).not.toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).not.toBeNull(); // Tab to the settings icon button. await page.keyboard.press( 'Tab' ); @@ -330,7 +356,11 @@ describe( 'Links', () => { // Expect the the escape key to dismiss the popover normally. await page.keyboard.press( 'Escape' ); - expect( await page.$( '.block-editor-url-popover' ) ).toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).toBeNull(); } ); it( 'can be modified using the keyboard once a link has been set', async () => { @@ -348,16 +378,30 @@ describe( 'Links', () => { // Deselect the link text by moving the caret to the end of the line // and the link popover should not be displayed. await page.keyboard.press( 'End' ); - expect( await page.$( '.block-editor-url-popover' ) ).toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).toBeNull(); // Move the caret back into the link text and the link popover // should be displayed. await page.keyboard.press( 'ArrowLeft' ); - expect( await page.$( '.block-editor-url-popover' ) ).not.toBeNull(); + expect( + await page.$( + '.components-popover__content .block-editor-link-control' + ) + ).not.toBeNull(); // Press Cmd+K to edit the link and the url-input should become // focused with the value previously inserted. await pressKeyWithModifier( 'primary', 'K' ); + await page.waitForSelector( + ':focus.block-editor-link-control__search-item-title' + ); + await page.keyboard.press( 'Tab' ); // Shift focus to "Edit" button + await page.keyboard.press( 'Enter' ); // Click "Edit" button + await waitForAutoFocus(); const activeElementParentClasses = await page.evaluate( () => Object.values( @@ -389,44 +433,6 @@ describe( 'Links', () => { ); } ); - it( 'link popover remains visible after a mouse drag event', async () => { - // Create some blocks so we have components with event handlers on the page - for ( let loop = 0; loop < 5; loop++ ) { - await insertBlock( 'Paragraph' ); - await page.keyboard.type( 'This is Gutenberg' ); - } - - // Focus on first paragraph, so the link popover will appear over the subsequent ones - await page.click( '[aria-label="Block navigation"]' ); - await page.click( '.block-editor-block-navigation__item button' ); - - // Select some text - await pressKeyWithModifier( 'shiftAlt', 'ArrowLeft' ); - - // Click on the Link button - await page.click( 'button[aria-label="Link"]' ); - // Wait for the URL field to auto-focus - await waitForAutoFocus(); - - // Click on the Link Settings button - await page.click( 'button[aria-label="Link settings"]' ); - - // Move mouse over the 'open in new tab' section, then click and drag - const settings = await page.$( '.block-editor-url-popover__settings' ); - const bounds = await settings.boundingBox(); - - await page.mouse.move( bounds.x, bounds.y ); - await page.mouse.down(); - await page.mouse.move( bounds.x + bounds.width / 2, bounds.y, { - steps: 10, - } ); - await page.mouse.up(); - - // The link popover should still be visible - const popover = await page.$$( '.block-editor-url-popover' ); - expect( popover ).toHaveLength( 1 ); - } ); - it( 'should contain a label when it should open in a new tab', async () => { await clickBlockAppender(); await page.keyboard.type( 'This is WordPress' ); @@ -435,19 +441,22 @@ describe( 'Links', () => { await pressKeyWithModifier( 'primary', 'k' ); await waitForAutoFocus(); await page.keyboard.type( 'w.org' ); - // Navigate to the settings toggle. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); - // Open settings. - await page.keyboard.press( 'Space' ); + + // Insert the link + await page.keyboard.press( 'Enter' ); + + // Navigate back to the popover + await pressKeyWithModifier( 'primary', 'k' ); + await page.waitForSelector( + '.components-popover__content .block-editor-link-control' + ); + // Navigate to the "Open in New Tab" checkbox. await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); + // Check the checkbox. await page.keyboard.press( 'Space' ); - // Navigate back to the input field. - await page.keyboard.press( 'Tab' ); - // Submit the form. - await page.keyboard.press( 'Enter' ); expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -463,36 +472,29 @@ describe( 'Links', () => { await page.keyboard.press( 'ArrowRight' ); // Edit link. await pressKeyWithModifier( 'primary', 'k' ); + await page.waitForSelector( + ':focus.block-editor-link-control__search-item-title' + ); + await page.keyboard.press( 'Tab' ); // Shift focus to "Edit" button + await page.keyboard.press( 'Enter' ); // Click "Edit" button await waitForAutoFocus(); await pressKeyWithModifier( 'primary', 'a' ); await page.keyboard.type( 'wordpress.org' ); - // Navigate to the settings toggle. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); - // Open settings. - await page.keyboard.press( 'Space' ); - // Navigate to the "Open in New Tab" checkbox. - await page.keyboard.press( 'Tab' ); - // Uncheck the checkbox. - await page.keyboard.press( 'Space' ); - // Navigate back to the input field. - await page.keyboard.press( 'Tab' ); - // Submit the form. + + // Update the link await page.keyboard.press( 'Enter' ); - // Navigate back to inputs to verify appears as changed. + // Navigate back to the popover await pressKeyWithModifier( 'primary', 'k' ); - await waitForAutoFocus(); - const link = await page.evaluate( () => document.activeElement.value ); - expect( link ).toBe( 'http://wordpress.org' ); + await page.waitForSelector( + '.components-popover__content .block-editor-link-control' + ); + + // Navigate to the "Open in New Tab" checkbox. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); + // Uncheck the checkbox. await page.keyboard.press( 'Space' ); - await page.keyboard.press( 'Tab' ); - const isChecked = await page.evaluate( - () => document.activeElement.checked - ); - expect( isChecked ).toBe( false ); expect( await getEditedPostContent() ).toMatchSnapshot(); } ); diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 1a63d9621e8059..8680d006e41e7e 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -149,16 +149,17 @@ export const link = { shortcutCharacter="k" /> ) } - + { ( this.state.addingLink || isActive ) && ( + + ) } ); } diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index aaeeb5006caa8e..58e767b2262620 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -1,10 +1,14 @@ +/** + * External dependencies + */ +import { uniqueId } from 'lodash'; + /** * WordPress dependencies */ +import { useMemo } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; -import { Component, createRef, useMemo } from '@wordpress/element'; -import { ToggleControl, withSpokenMessages } from '@wordpress/components'; -import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; +import { withSpokenMessages, Popover } from '@wordpress/components'; import { prependHTTP } from '@wordpress/url'; import { create, @@ -14,20 +18,39 @@ import { getTextContent, slice, } from '@wordpress/rich-text'; -import { URLPopover } from '@wordpress/block-editor'; +import { __experimentalLinkControl as LinkControl } from '@wordpress/block-editor'; /** * Internal dependencies */ import { createLinkFormat, isValidHref } from './utils'; -const stopKeyPropagation = ( event ) => event.stopPropagation(); +function InlineLinkUI( { + isActive, + activeAttributes, + addingLink, + value, + onChange, + onFocus, + speak, + stopAddingLink, +} ) { + /** + * A unique key is generated when switching between editing and not editing + * a link, based on: + * + * - This component may be rendered _either_ when a link is active _or_ + * when adding or editing a link. + * - It's only desirable to shift focus into the Popover when explicitly + * adding or editing a link, not when in the inline boundary of a link. + * - Focus behavior can only be controlled on a Popover at the time it + * mounts, so a new instance of the component must be mounted to + * programmatically enact the focusOnMount behavior. + * + * @type {string} + */ + const mountingKey = useMemo( uniqueId, [ addingLink ] ); -function isShowingInput( props, state ) { - return props.addingLink || state.editLink; -} - -const URLPopoverAtLink = ( { isActive, addingLink, value, ...props } ) => { const anchorRef = useMemo( () => { const selection = window.getSelection(); @@ -37,7 +60,7 @@ const URLPopoverAtLink = ( { isActive, addingLink, value, ...props } ) => { const range = selection.getRangeAt( 0 ); - if ( addingLink ) { + if ( addingLink && ! isActive ) { return range; } @@ -51,120 +74,28 @@ const URLPopoverAtLink = ( { isActive, addingLink, value, ...props } ) => { } return element.closest( 'a' ); - }, [ isActive, addingLink, value.start, value.end ] ); - - if ( ! anchorRef ) { - return null; - } - - return ; -}; - -class InlineLinkUI extends Component { - constructor() { - super( ...arguments ); - - this.editLink = this.editLink.bind( this ); - this.submitLink = this.submitLink.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); - this.onChangeInputValue = this.onChangeInputValue.bind( this ); - this.setLinkTarget = this.setLinkTarget.bind( this ); - this.onFocusOutside = this.onFocusOutside.bind( this ); - this.resetState = this.resetState.bind( this ); - this.autocompleteRef = createRef(); - - this.state = { - opensInNewWindow: false, - inputValue: '', - }; - } - - static getDerivedStateFromProps( props, state ) { - const { - activeAttributes: { url, target }, - } = props; - const opensInNewWindow = target === '_blank'; - - if ( ! isShowingInput( props, state ) ) { - const update = {}; - if ( url !== state.inputValue ) { - update.inputValue = url; - } - - if ( opensInNewWindow !== state.opensInNewWindow ) { - update.opensInNewWindow = opensInNewWindow; - } - return Object.keys( update ).length ? update : null; - } - - return null; - } - - onKeyDown( event ) { - if ( - [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( - event.keyCode - ) > -1 - ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - } - - onChangeInputValue( inputValue ) { - this.setState( { inputValue } ); - } + }, [ addingLink, value.start, value.end ] ); - setLinkTarget( opensInNewWindow ) { - const { - activeAttributes: { url = '' }, - value, - onChange, - } = this.props; + const linkValue = { + url: activeAttributes.url, + opensInNewTab: activeAttributes.target === '_blank', + }; - this.setState( { opensInNewWindow } ); - - // Apply now if URL is not being edited. - if ( ! isShowingInput( this.props, this.state ) ) { - const selectedText = getTextContent( slice( value ) ); - - onChange( - applyFormat( - value, - createLinkFormat( { - url, - opensInNewWindow, - text: selectedText, - } ) - ) - ); - } - } - - editLink( event ) { - this.setState( { editLink: true } ); - event.preventDefault(); - } - - submitLink( event ) { - const { isActive, value, onChange, onFocus, speak } = this.props; - const { inputValue, opensInNewWindow } = this.state; - const url = prependHTTP( inputValue ); + function onChangeLink( nextValue ) { + const newUrl = prependHTTP( nextValue.url ); const selectedText = getTextContent( slice( value ) ); const format = createLinkFormat( { - url, - opensInNewWindow, + url: newUrl, + opensInNewWindow: nextValue.opensInNewTab, text: selectedText, } ); - event.preventDefault(); - if ( isCollapsed( value ) && ! isActive ) { const toInsert = applyFormat( - create( { text: url } ), + create( { text: newUrl } ), format, 0, - url.length + newUrl.length ); onChange( insert( value, toInsert ) ); } else { @@ -172,10 +103,9 @@ class InlineLinkUI extends Component { } onFocus(); + stopAddingLink(); - this.resetState(); - - if ( ! isValidHref( url ) ) { + if ( ! isValidHref( newUrl ) ) { speak( __( 'Warning: the link has been inserted but may have errors. Please test it.' @@ -189,84 +119,17 @@ class InlineLinkUI extends Component { } } - onFocusOutside() { - // The autocomplete suggestions list renders in a separate popover (in a portal), - // so onFocusOutside fails to detect that a click on a suggestion occurred in the - // LinkContainer. Detect clicks on autocomplete suggestions using a ref here, and - // return to avoid the popover being closed. - const autocompleteElement = this.autocompleteRef.current; - if ( - autocompleteElement && - autocompleteElement.contains( document.activeElement ) - ) { - return; - } - - this.resetState(); - } - - resetState() { - this.props.stopAddingLink(); - this.setState( { editLink: false } ); - } - - render() { - const { - isActive, - activeAttributes: { url }, - addingLink, - value, - } = this.props; - - if ( ! isActive && ! addingLink ) { - return null; - } - - const { inputValue, opensInNewWindow } = this.state; - const showInput = isShowingInput( this.props, this.state ); - - return ( - ( - - ) } - > - { showInput ? ( - - ) : ( - - ) } - - ); - } + return ( + + + + ); } export default withSpokenMessages( InlineLinkUI ); diff --git a/packages/format-library/src/link/test/inline.js b/packages/format-library/src/link/test/inline.js index cc59087e8c429b..b3be2c1de0720b 100644 --- a/packages/format-library/src/link/test/inline.js +++ b/packages/format-library/src/link/test/inline.js @@ -12,28 +12,4 @@ describe( 'InlineLinkUI', () => { const wrapper = shallow( ); expect( wrapper ).toBeTruthy(); } ); - - it( 'should set state.opensInNewWindow to false by default', () => { - const wrapper = shallow( - - ).dive(); - - expect( wrapper.state( 'opensInNewWindow' ) ).toEqual( false ); - } ); - - it( 'should set state.opensInNewWindow to true if props.activeAttributes.target is _blank', () => { - const givenProps = { - addingLink: false, - activeAttributes: { - url: 'http://www.google.com', - target: '_blank', - }, - }; - - const wrapper = shallow( - - ).dive(); - wrapper.setProps( givenProps ); - expect( wrapper.state( 'opensInNewWindow' ) ).toEqual( true ); - } ); } );