From 6c591f4518218e992ed85ef3d0e15a5e92f57af8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 10 Feb 2020 08:37:34 -0500 Subject: [PATCH] Block Editor: LinkControl: Incorporate settings in edit state (#20052) * Block Editor: LinkControl: Incorporate settings in edit state * Block Editor: LinkControl: Add forceIsEditingLink prop support * Format Library: Force LinkControl editing state when adding link * E2E Tests: Update end-to-end tests for expected link keyboard workflow * Revert "Block Editor: LinkControl: Incorporate settings in edit state" This reverts commit 559656662f5047f3214cbbda9d9dce56c41e8334. * Block Library: LinkControl: Always show settings drawer * Format Library: Avoid shifting focus back to paragraph on toggle * Components: Extract spinner size as base variable Allow for reuse in computing dimensions in other stylesheets * Block Editor: LinkControl: Add Submit button as action * Block Editor: LinkControl: Remove Reset button * Block Editor: LinkControl: Remove `disabled` from submit button Instead inherit that the form cannot be submitted since the rendered URLInput will apply a `required` attribute, preventing submission of the form, while still displaying the associated help text "You must fill out this field" * Block Editor: LinkControl: Pad horizontally to match vertical * Format Library: Return paragraph selection on link edit cancel * Block Editor: LinkControl: Infer absence of value setting as unchecked * Format Library: Hold "Open in New Tab" during insert * Format Library: Refactor, comment inline link state flow --- packages/base-styles/_variables.scss | 9 +- .../src/components/link-control/README.md | 7 + .../src/components/link-control/index.js | 34 +++-- .../components/link-control/search-input.js | 19 ++- .../link-control/settings-drawer.js | 2 +- .../src/components/link-control/style.scss | 43 ++++-- .../test/__snapshots__/index.js.snap | 2 +- .../src/components/link-control/test/index.js | 123 ++++++++++-------- packages/components/src/spinner/style.scss | 14 +- .../various/__snapshots__/links.test.js.snap | 20 ++- .../specs/editor/various/links.test.js | 73 ++++++----- packages/format-library/src/link/index.js | 3 +- packages/format-library/src/link/inline.js | 54 +++++++- 13 files changed, 265 insertions(+), 138 deletions(-) diff --git a/packages/base-styles/_variables.scss b/packages/base-styles/_variables.scss index 2f6776824c45b..5cb403ffd96a1 100644 --- a/packages/base-styles/_variables.scss +++ b/packages/base-styles/_variables.scss @@ -48,12 +48,8 @@ $content-width: 580px; // This is optimized for 70 characters. // Block UI $border-width: 1px; $block-controls-height: 36px; -$icon-button-size: 36px; -$icon-button-size-small: 24px; $inserter-tabs-height: 36px; $block-toolbar-height: $block-controls-height + $border-width; -$resize-handler-size: 15px; -$resize-handler-container-size: $resize-handler-size + ($grid-size-small * 2); // Make the resize handle container larger so there's a larger grabbable area. // Blocks $block-left-border-width: $border-width * 3; @@ -82,6 +78,11 @@ $block-selected-vertical-margin-child: $block-edge-to-content; // Buttons & UI Widgets $radius-round-rectangle: 4px; $radius-round: 50%; +$icon-button-size: 36px; +$icon-button-size-small: 24px; +$resize-handler-size: 15px; +$resize-handler-container-size: $resize-handler-size + ($grid-size-small * 2); // Make the resize handle container larger so there's a larger grabbable area. +$spinner-size: 18px; // Widgets screen $widget-area-width: 700px; diff --git a/packages/block-editor/src/components/link-control/README.md b/packages/block-editor/src/components/link-control/README.md index 64bde7b5658ce..4b590885ae8be 100644 --- a/packages/block-editor/src/components/link-control/README.md +++ b/packages/block-editor/src/components/link-control/README.md @@ -64,3 +64,10 @@ Value change handler, called with the updated value if the user selects a new li - Default: `false` Whether to present initial suggestions immediately. + +### forceIsEditingLink + +- Type: `boolean` +- Required: No + +If passed as either `true` or `false`, controls the internal editing state of the component to respective show or not show the URL input field. diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 0dc0f55a82591..3ef9c75fa8f26 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -75,8 +75,9 @@ import LinkControlSearchInput from './search-input'; * * @property {(WPLinkControlSetting[])=} settings An array of settings objects. Each object will used to * render a `ToggleControl` for that setting. - * @property {(search:string)=>Promise=} fetchSearchSuggestions Fetches suggestions for a given search term, - * returning a promise resolving once fetch is complete. + * @property {boolean=} forceIsEditingLink If passed as either `true` or `false`, controls the + * internal editing state of the component to respective + * show or not show the URL input field. * @property {WPLinkControlValue=} value Current link value. * @property {WPLinkControlOnChangeProp=} onChange Value change handler, called with the updated value if * the user selects a new link or updates settings. @@ -95,6 +96,7 @@ function LinkControl( { settings, onChange = noop, showInitialSuggestions, + forceIsEditingLink, } ) { const wrapperNode = useRef(); const instanceId = useInstanceId( LinkControl ); @@ -102,7 +104,9 @@ function LinkControl( { ( value && value.url ) || '' ); const [ isEditingLink, setIsEditingLink ] = useState( - ! value || ! value.url + forceIsEditingLink !== undefined + ? forceIsEditingLink + : ! value || ! value.url ); const isEndingEditWithFocus = useRef( false ); const { fetchSearchSuggestions } = useSelect( ( select ) => { @@ -115,6 +119,15 @@ function LinkControl( { const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ) ) ) || ''; + useEffect( () => { + if ( + forceIsEditingLink !== undefined && + forceIsEditingLink !== isEditingLink + ) { + setIsEditingLink( forceIsEditingLink ); + } + }, [ forceIsEditingLink ] ); + useEffect( () => { // When `isEditingLink` is set to `false`, a focus loss could occur // since the link input may be removed from the DOM. To avoid this, @@ -150,10 +163,6 @@ function LinkControl( { setInputValue( val ); }; - const resetInput = () => { - setInputValue( '' ); - }; - const handleDirectEntry = ( val ) => { let type = 'URL'; @@ -316,7 +325,6 @@ function LinkControl( { } } renderSuggestions={ renderSearchResults } fetchSuggestions={ getSearchHandler } - onReset={ resetInput } showInitialSuggestions={ showInitialSuggestions } /> ) : ( @@ -359,13 +367,13 @@ function LinkControl( { { __( 'Edit' ) } - ) } + ); } diff --git a/packages/block-editor/src/components/link-control/search-input.js b/packages/block-editor/src/components/link-control/search-input.js index 476fcf7830f6b..44da93ab9f2ea 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -5,7 +5,6 @@ import { useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Button } from '@wordpress/components'; import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; -import { close } from '@wordpress/icons'; /** * Internal dependencies @@ -36,7 +35,6 @@ const LinkControlSearchInput = ( { onSelect, renderSuggestions, fetchSuggestions, - onReset, showInitialSuggestions, } ) => { const [ selectedSuggestion, setSelectedSuggestion ] = useState(); @@ -74,15 +72,14 @@ const LinkControlSearchInput = ( { __experimentalHandleURLSuggestions={ true } __experimentalShowInitialSuggestions={ showInitialSuggestions } /> - - "`; +exports[`Basic rendering should render 1`] = `"
Currently selected link settings
"`; 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 65d7729c973dc..2776e4ddf078a 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -64,6 +64,69 @@ describe( 'Basic rendering', () => { expect( searchInput ).not.toBeNull(); expect( container.innerHTML ).toMatchSnapshot(); } ); + + describe( 'forceIsEditingLink', () => { + const isEditing = () => + !! container.querySelector( 'input[aria-label="URL"]' ); + + it( 'undefined', () => { + act( () => { + render( + , + container + ); + } ); + + expect( isEditing() ).toBe( false ); + } ); + + it( 'true', () => { + act( () => { + render( + , + container + ); + } ); + + expect( isEditing() ).toBe( true ); + } ); + + it( 'false', () => { + act( () => { + render( + , + container + ); + } ); + + // Click the "Edit" button to trigger into the editing mode. + const editButton = container.querySelector( + '.block-editor-link-control__search-item-action--edit' + ); + act( () => { + Simulate.click( editButton ); + } ); + + expect( isEditing() ).toBe( true ); + + // If passed `forceIsEditingLink` of `false` while editing, should + // forcefully reset to the preview state. + act( () => { + render( + , + container + ); + } ); + + expect( isEditing() ).toBe( false ); + } ); + } ); } ); describe( 'Searching for a link', () => { @@ -217,56 +280,6 @@ describe( 'Searching for a link', () => { ); } ); - - it( 'should reset the input field and the search results when search term is cleared or reset', async () => { - const searchTerm = 'Hello world'; - - act( () => { - render( , container ); - } ); - - let searchResultElements; - let searchInput; - - // Search Input UI - searchInput = container.querySelector( 'input[aria-label="URL"]' ); - - // Simulate searching for a term - act( () => { - Simulate.change( searchInput, { target: { value: searchTerm } } ); - } ); - - // fetchFauxEntitySuggestions resolves on next "tick" of event loop - await eventLoopTick(); - - // TODO: select these by aria relationship to autocomplete rather than arbitary selector. - searchResultElements = container.querySelectorAll( - '[role="listbox"] [role="option"]' - ); - - // Check we have definitely rendered some suggestions - expect( searchResultElements ).toHaveLength( - fauxEntitySuggestions.length - ); - - // Grab the reset button now it's available - const resetUI = container.querySelector( '[aria-label="Reset"]' ); - - act( () => { - Simulate.click( resetUI ); - } ); - - await eventLoopTick(); - - // TODO: select these by aria relationship to autocomplete rather than arbitary selector. - searchResultElements = container.querySelectorAll( - '[role="listbox"] [role="option"]' - ); - searchInput = container.querySelector( 'input[aria-label="URL"]' ); - - expect( searchInput.value ).toBe( '' ); - expect( searchResultElements ).toHaveLength( 0 ); - } ); } ); describe( 'Manual link entry', () => { @@ -458,13 +471,15 @@ describe( 'Default search suggestions', () => { expect( mockFetchSearchSuggestions ).not.toHaveBeenCalled(); - // // Reset the search to empty and check the initial suggestions are now shown. - // - const resetUI = container.querySelector( '[aria-label="Reset"]' ); + const searchInput = container.querySelector( + 'input[aria-label="URL"]' + ); act( () => { - Simulate.click( resetUI ); + Simulate.change( searchInput, { + target: { value: '' }, + } ); } ); await eventLoopTick(); diff --git a/packages/components/src/spinner/style.scss b/packages/components/src/spinner/style.scss index cdd363ab104ef..1fba4c387d2e2 100644 --- a/packages/components/src/spinner/style.scss +++ b/packages/components/src/spinner/style.scss @@ -1,8 +1,8 @@ .components-spinner { display: inline-block; background-color: $dark-gray-200; - width: 18px; - height: 18px; + width: $spinner-size; + height: $spinner-size; opacity: 0.7; margin: 5px 11px 0; border-radius: 100%; @@ -13,12 +13,12 @@ content: ""; position: absolute; background-color: $white; - top: 3px; - left: 3px; - width: 4px; - height: 4px; + top: ( $spinner-size - ( $spinner-size * ( 2 / 3 ) ) ) / 2; + left: ( $spinner-size - ( $spinner-size * ( 2 / 3 ) ) ) / 2; + width: ( $spinner-size / 4.5 ); + height: ( $spinner-size / 4.5 ); border-radius: 100%; - transform-origin: 6px 6px; + transform-origin: ( $spinner-size / 3 ) ( $spinner-size / 3 ); animation: components-spinner__animation 1s infinite linear; /* rtl:end:ignore */ } diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap index 292ac9e5781a9..cf6bb746c5603 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap @@ -1,5 +1,11 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Links allows use of escape key to dismiss the url popover 1`] = ` +" +

This is Gutenberg.

+" +`; + exports[`Links can be created by selecting text and clicking Link 1`] = ` "

This is Gutenberg

@@ -8,7 +14,13 @@ exports[`Links can be created by selecting text and clicking Link 1`] = ` exports[`Links can be created by selecting text and using keyboard shortcuts 1`] = ` " -

This is Gutenberg

+

This is Gutenberg

+" +`; + +exports[`Links can be created by selecting text and using keyboard shortcuts 2`] = ` +" +

This is Gutenberg

" `; @@ -36,6 +48,12 @@ exports[`Links can be edited with collapsed selection 1`] = ` " `; +exports[`Links can be modified using the keyboard once a link has been set 1`] = ` +" +

This is Gutenberg.

+" +`; + exports[`Links can be removed 1`] = ` "

This is Gutenberg

diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index 0f21d248a8650..797de506e7f5e 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -67,7 +67,22 @@ describe( 'Links', () => { // Type a URL await page.keyboard.type( 'https://wordpress.org/gutenberg' ); - // Press Enter to apply the link + // Navigate to and toggle the "Open in New Tab" checkbox. + await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Space' ); + + // Toggle should still have focus and be checked. + await page.waitForSelector( + ':focus:checked.components-form-toggle__input' + ); + + // Ensure that the contents of the post have not been changed, since at + // this point the link is still not inserted. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + // Tab back to the Submit and apply the link + await pressKeyWithModifier( 'shift', 'Tab' ); await page.keyboard.press( 'Enter' ); // The link should have been inserted @@ -320,6 +335,12 @@ describe( 'Links', () => { ) ).toBeNull(); + // Confirm that selection is returned to where it was before launching + // the link editor, with "Gutenberg" as an uncollapsed selection. + await page.keyboard.press( 'ArrowRight' ); + await page.keyboard.type( '.' ); + expect( await getEditedPostContent() ).toMatchSnapshot(); + // Press Cmd+K to insert a link await pressKeyWithModifier( 'primary', 'K' ); @@ -350,7 +371,7 @@ describe( 'Links', () => { ) ).not.toBeNull(); - // Tab to the settings icon button. + // Tab to the "Open in New Tab" toggle. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); @@ -396,25 +417,22 @@ describe( 'Links', () => { // 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( - document.activeElement.parentElement.parentElement.classList - ) - ); - expect( activeElementParentClasses ).toContain( - 'block-editor-url-input' + const isInURLInput = await page.evaluate( + () => !! document.activeElement.closest( '.block-editor-url-input' ) ); + expect( isInURLInput ).toBe( true ); const activeElementValue = await page.evaluate( () => document.activeElement.value ); expect( activeElementValue ).toBe( URL ); + + // Confirm that submitting the input without any changes keeps the same + // value and moves focus back to the paragraph. + await page.keyboard.press( 'Enter' ); + await page.keyboard.press( 'ArrowRight' ); + await page.keyboard.type( '.' ); + expect( await getEditedPostContent() ).toMatchSnapshot(); } ); it( 'adds an assertive message for screenreader users when an invalid link is set', async () => { @@ -447,17 +465,21 @@ describe( 'Links', () => { // Navigate back to the popover await pressKeyWithModifier( 'primary', 'k' ); - await page.waitForSelector( - '.components-popover__content .block-editor-link-control' - ); + await waitForAutoFocus(); - // Navigate to the "Open in New Tab" checkbox. + // Navigate to and toggle the "Open in New Tab" checkbox. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); - - // Check the checkbox. await page.keyboard.press( 'Space' ); + // Confirm that focus was not prematurely returned to the paragraph on + // a changing value of the setting. + await page.waitForSelector( ':focus.components-form-toggle__input' ); + + // Close dialog. Expect that "Open in New Tab" would have been applied + // immediately. + await page.keyboard.press( 'Escape' ); + expect( await getEditedPostContent() ).toMatchSnapshot(); // Regression Test: This verifies that the UI is updated according to @@ -472,11 +494,6 @@ 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' ); @@ -486,9 +503,7 @@ describe( 'Links', () => { // Navigate back to the popover await pressKeyWithModifier( 'primary', 'k' ); - await page.waitForSelector( - '.components-popover__content .block-editor-link-control' - ); + await waitForAutoFocus(); // Navigate to the "Open in New Tab" checkbox. await page.keyboard.press( 'Tab' ); diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 07f41055f7528..a48a3acc7c293 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -98,6 +98,7 @@ export const link = { stopAddingLink() { this.setState( { addingLink: false } ); + this.props.onFocus(); } onRemoveFormat() { @@ -113,7 +114,6 @@ export const link = { activeAttributes, value, onChange, - onFocus, } = this.props; return ( @@ -158,7 +158,6 @@ export const link = { activeAttributes={ activeAttributes } value={ value } onChange={ onChange } - onFocus={ onFocus } /> ) } diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 58e767b226262..2d9eb18df1578 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -6,7 +6,7 @@ import { uniqueId } from 'lodash'; /** * WordPress dependencies */ -import { useMemo } from '@wordpress/element'; +import { useMemo, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { withSpokenMessages, Popover } from '@wordpress/components'; import { prependHTTP } from '@wordpress/url'; @@ -31,7 +31,6 @@ function InlineLinkUI( { addingLink, value, onChange, - onFocus, speak, stopAddingLink, } ) { @@ -51,6 +50,16 @@ function InlineLinkUI( { */ const mountingKey = useMemo( uniqueId, [ addingLink ] ); + /** + * Pending settings to be applied to the next link. When inserting a new + * link, toggle values cannot be applied immediately, because there is not + * yet a link for them to apply to. Thus, they are maintained in a state + * value until the time that the link can be inserted or edited. + * + * @type {[Object|undefined,Function]} + */ + const [ nextLinkValue, setNextLinkValue ] = useState(); + const anchorRef = useMemo( () => { const selection = window.getSelection(); @@ -79,9 +88,37 @@ function InlineLinkUI( { const linkValue = { url: activeAttributes.url, opensInNewTab: activeAttributes.target === '_blank', + ...nextLinkValue, }; function onChangeLink( nextValue ) { + // Merge with values from state, both for the purpose of assigning the + // next state value, and for use in constructing the new link format if + // the link is ready to be applied. + nextValue = { + ...nextLinkValue, + ...nextValue, + }; + + // LinkControl calls `onChange` immediately upon the toggling a setting. + const didToggleSetting = + linkValue.opensInNewTab !== nextValue.opensInNewTab && + linkValue.url === nextValue.url; + + // If change handler was called as a result of a settings change during + // link insertion, it must be held in state until the link is ready to + // be applied. + const didToggleSettingForNewLink = + didToggleSetting && nextValue.url === undefined; + + // If link will be assigned, the state value can be considered flushed. + // Otherwise, persist the pending changes. + setNextLinkValue( didToggleSettingForNewLink ? nextValue : undefined ); + + if ( didToggleSettingForNewLink ) { + return; + } + const newUrl = prependHTTP( nextValue.url ); const selectedText = getTextContent( slice( value ) ); const format = createLinkFormat( { @@ -102,8 +139,11 @@ function InlineLinkUI( { onChange( applyFormat( value, format ) ); } - onFocus(); - stopAddingLink(); + // Focus should only be shifted back to the formatted segment when the + // URL is submitted. + if ( ! didToggleSetting ) { + stopAddingLink(); + } if ( ! isValidHref( newUrl ) ) { speak( @@ -127,7 +167,11 @@ function InlineLinkUI( { onClose={ stopAddingLink } position="bottom center" > - + ); }