From 572224973ecaaf72e640f6237f72fc2d1bd40815 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 5 Feb 2020 12:05:51 -0500 Subject: [PATCH 01/16] Block Editor: LinkControl: Incorporate settings in edit state --- .../src/components/link-control/index.js | 19 +++----- .../components/link-control/search-input.js | 44 ++++++++++++++----- .../link-control/settings-drawer.js | 4 +- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 018d8386c153e..fcce9fccd48af 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -30,7 +30,6 @@ import { focus } from '@wordpress/dom'; /** * Internal dependencies */ -import LinkControlSettingsDrawer from './settings-drawer'; import LinkControlSearchItem from './search-item'; import LinkControlSearchInput from './search-input'; @@ -150,10 +149,6 @@ function LinkControl( { setInputValue( val ); }; - const resetInput = () => { - setInputValue( '' ); - }; - const handleDirectEntry = ( val ) => { let type = 'URL'; @@ -308,15 +303,16 @@ function LinkControl( { > { isEditingLink || ! value ? ( { - onChange( { ...value, ...suggestion } ); + onSelect={ ( nextValue ) => { + onChange( nextValue ); stopEditing(); } } + settings={ settings } renderSuggestions={ renderSearchResults } fetchSuggestions={ getSearchHandler } - onReset={ resetInput } showInitialSuggestions={ showInitialSuggestions } /> ) : ( @@ -359,11 +355,6 @@ 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..26a6957282892 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -5,12 +5,13 @@ 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'; +import { chevronDown, chevronUp } from '@wordpress/icons'; /** * Internal dependencies */ import { URLInput } from '../'; +import LinkControlSettingsDrawer from './settings-drawer'; const handleLinkControlOnKeyDown = ( event ) => { const { keyCode } = event; @@ -32,14 +33,17 @@ const handleLinkControlOnKeyPress = ( event ) => { const LinkControlSearchInput = ( { value, + inputValue, onChange, onSelect, + settings, renderSuggestions, fetchSuggestions, - onReset, showInitialSuggestions, } ) => { const [ selectedSuggestion, setSelectedSuggestion ] = useState(); + const [ isSettingsExpanded, setIsSettingsExpanded ] = useState( false ); + const [ pendingSettingsChanges, setPendingSettingsChanges ] = useState(); const selectItemHandler = ( selection, suggestion ) => { onChange( selection ); @@ -51,15 +55,19 @@ const LinkControlSearchInput = ( { event.preventDefault(); // Interpret the selected value as either the selected suggestion, if - // exists, or otherwise the current input value as entered. - onSelect( selectedSuggestion || { url: value } ); + // exists, or otherwise the current input value as entered. Settings + // changes are held in state and merged into the changed value. + onSelect( { + ...pendingSettingsChanges, + ...( selectedSuggestion || { url: inputValue } ), + } ); } return (
{ if ( event.keyCode === ENTER ) { @@ -74,15 +82,27 @@ const LinkControlSearchInput = ( { __experimentalHandleURLSuggestions={ true } __experimentalShowInitialSuggestions={ showInitialSuggestions } /> - + + ) } 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 26a6957282892..476fcf7830f6b 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -5,13 +5,12 @@ 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 { chevronDown, chevronUp } from '@wordpress/icons'; +import { close } from '@wordpress/icons'; /** * Internal dependencies */ import { URLInput } from '../'; -import LinkControlSettingsDrawer from './settings-drawer'; const handleLinkControlOnKeyDown = ( event ) => { const { keyCode } = event; @@ -33,17 +32,14 @@ const handleLinkControlOnKeyPress = ( event ) => { const LinkControlSearchInput = ( { value, - inputValue, onChange, onSelect, - settings, renderSuggestions, fetchSuggestions, + onReset, showInitialSuggestions, } ) => { const [ selectedSuggestion, setSelectedSuggestion ] = useState(); - const [ isSettingsExpanded, setIsSettingsExpanded ] = useState( false ); - const [ pendingSettingsChanges, setPendingSettingsChanges ] = useState(); const selectItemHandler = ( selection, suggestion ) => { onChange( selection ); @@ -55,19 +51,15 @@ const LinkControlSearchInput = ( { event.preventDefault(); // Interpret the selected value as either the selected suggestion, if - // exists, or otherwise the current input value as entered. Settings - // changes are held in state and merged into the changed value. - onSelect( { - ...pendingSettingsChanges, - ...( selectedSuggestion || { url: inputValue } ), - } ); + // exists, or otherwise the current input value as entered. + onSelect( selectedSuggestion || { url: value } ); } return (
{ if ( event.keyCode === ENTER ) { @@ -82,27 +74,15 @@ const LinkControlSearchInput = ( { __experimentalHandleURLSuggestions={ true } __experimentalShowInitialSuggestions={ showInitialSuggestions } /> + - - ) } + ); } diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index e62045b594e97..cc04ec96a5790 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -350,7 +350,7 @@ describe( 'Links', () => { ) ).not.toBeNull(); - // Tab to the "Advanced Settings" toggle. + // Tab to the "Open in New Tab" toggle. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); @@ -443,17 +443,14 @@ describe( 'Links', () => { await pressKeyWithModifier( 'primary', 'k' ); await waitForAutoFocus(); - // Navigate to and expand the "Advanced Settings" toggle. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Space' ); - // 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' ); - // Submit change to "Open in New Tab". - await page.keyboard.press( 'Enter' ); + // Close dialog. Expect that "Open in New Tab" would have been applied + // immediately. + await page.keyboard.press( 'Escape' ); expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -473,18 +470,19 @@ describe( 'Links', () => { await pressKeyWithModifier( 'primary', 'a' ); await page.keyboard.type( 'wordpress.org' ); - // Navigate to and expand the "Advanced Settings" toggle. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Space' ); + // Update the link + await page.keyboard.press( 'Enter' ); - // Navigate to and uncheck the "Open in New Tab" checkbox. + // Navigate back to the popover + await pressKeyWithModifier( 'primary', 'k' ); + await waitForAutoFocus(); + + // 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' ); - // Update the link. - await page.keyboard.press( 'Enter' ); - expect( await getEditedPostContent() ).toMatchSnapshot(); } ); } ); From 74d659b9c35c578551ef268850b21471ae5070bd Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 6 Feb 2020 14:32:41 -0500 Subject: [PATCH 07/16] Format Library: Avoid shifting focus back to paragraph on toggle --- .../various/__snapshots__/links.test.js.snap | 6 +++++ .../specs/editor/various/links.test.js | 24 +++++++++++++------ packages/format-library/src/link/inline.js | 12 ++++++++-- 3 files changed, 33 insertions(+), 9 deletions(-) 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..73f05ff6816aa 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 @@ -36,6 +36,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 cc04ec96a5790..b871af670a110 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -397,18 +397,21 @@ describe( 'Links', () => { // focused with the value previously inserted. await pressKeyWithModifier( 'primary', 'K' ); 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 () => { @@ -448,6 +451,10 @@ describe( 'Links', () => { await page.keyboard.press( 'Tab' ); 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' ); @@ -485,4 +492,7 @@ describe( 'Links', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + // TODO: Assert: Verify that pressing Escape while editing a link returns + // focus to the correct selection in the paragraph. } ); diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 3917386196321..cc5d3636c51f1 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -102,8 +102,16 @@ function InlineLinkUI( { onChange( applyFormat( value, format ) ); } - onFocus(); - stopAddingLink(); + // LinkControl calls `onChange` immediately upon the toggling of any + // settings, but focus should only be shifted back to the formatted + // segment when the URL is submitted. + const didToggleSetting = + linkValue.opensInNewTab !== nextValue.opensInNewTab && + linkValue.url === nextValue.url; + if ( ! didToggleSetting ) { + onFocus(); + stopAddingLink(); + } if ( ! isValidHref( newUrl ) ) { speak( From 4d43e07c4e51bf09a45b84be2307de23bed066bc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 28 Jan 2020 15:30:02 -0500 Subject: [PATCH 08/16] Components: Extract spinner size as base variable Allow for reuse in computing dimensions in other stylesheets --- packages/base-styles/_variables.scss | 9 +++++---- packages/components/src/spinner/style.scss | 14 +++++++------- 2 files changed, 12 insertions(+), 11 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/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 */ } From 5ff4106e3e40436e809606cff4b9eb0c6f6d1626 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 30 Jan 2020 14:48:55 -0500 Subject: [PATCH 09/16] Block Editor: LinkControl: Add Submit button as action --- .../components/link-control/search-input.js | 27 ++++++++----- .../src/components/link-control/style.scss | 39 ++++++++++++++----- .../test/__snapshots__/index.js.snap | 2 +- 3 files changed, 47 insertions(+), 21 deletions(-) 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..532863aeddfa7 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 @@ -74,15 +73,23 @@ const LinkControlSearchInput = ( { __experimentalHandleURLSuggestions={ true } __experimentalShowInitialSuggestions={ showInitialSuggestions } /> - - "`; +exports[`Basic rendering should render 1`] = `""`; From 74dd122a3873541ab277ff97de6c4f0adad4cde8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 3 Feb 2020 18:41:44 -0500 Subject: [PATCH 10/16] Block Editor: LinkControl: Remove Reset button --- .../src/components/link-control/index.js | 5 -- .../components/link-control/search-input.js | 9 --- .../src/components/link-control/style.scss | 6 +- .../test/__snapshots__/index.js.snap | 2 +- .../src/components/link-control/test/index.js | 60 ++----------------- .../various/__snapshots__/links.test.js.snap | 6 ++ .../specs/editor/various/links.test.js | 9 ++- 7 files changed, 23 insertions(+), 74 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 55bf239e40607..604c0b965ce53 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -163,10 +163,6 @@ function LinkControl( { setInputValue( val ); }; - const resetInput = () => { - setInputValue( '' ); - }; - const handleDirectEntry = ( val ) => { let type = 'URL'; @@ -329,7 +325,6 @@ function LinkControl( { } } renderSuggestions={ renderSearchResults } fetchSuggestions={ getSearchHandler } - onReset={ resetInput } showInitialSuggestions={ showInitialSuggestions } /> ) : ( 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 532863aeddfa7..695cdae717bef 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -35,7 +35,6 @@ const LinkControlSearchInput = ( { onSelect, renderSuggestions, fetchSuggestions, - onReset, showInitialSuggestions, } ) => { const [ selectedSuggestion, setSelectedSuggestion ] = useState(); @@ -81,14 +80,6 @@ const LinkControlSearchInput = ( { icon="editor-break" className="block-editor-link-control__search-submit" /> - "`; +exports[`Basic rendering should render 1`] = `""`; 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 c58459537cbe6..2776e4ddf078a 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -280,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', () => { @@ -521,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/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap index 73f05ff6816aa..817be306f4bbf 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

diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index b871af670a110..9ad716edca01a 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -320,6 +320,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' ); @@ -492,7 +498,4 @@ describe( 'Links', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - - // TODO: Assert: Verify that pressing Escape while editing a link returns - // focus to the correct selection in the paragraph. } ); From 770e72dca8e72241708015ec6fc275ac1e236609 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 3 Feb 2020 18:43:12 -0500 Subject: [PATCH 11/16] 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/src/components/link-control/search-input.js | 1 - .../components/link-control/test/__snapshots__/index.js.snap | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) 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 695cdae717bef..44da93ab9f2ea 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -74,7 +74,6 @@ const LinkControlSearchInput = ( { />
"`; +exports[`Basic rendering should render 1`] = `""`; From b328d20684720fc3592ef4872997dc75fc8630e1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 6 Feb 2020 14:46:04 -0500 Subject: [PATCH 12/16] Block Editor: LinkControl: Pad horizontally to match vertical --- packages/block-editor/src/components/link-control/style.scss | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 58e9644801e36..959e1f30c1126 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -42,9 +42,11 @@ $block-editor-link-control-number-of-actions: 1; * - Border (1px) * - Vertically, for the difference in height between the input (40px) and * the icon buttons. + * - Horizontally, pad to the minimum of: default input padding, or the + * equivalent of the vertical padding. */ top: $grid-size-large + 1px + ( ( 40px - $icon-button-size ) / 2 ); - right: $grid-size-large + 1px; + right: $grid-size-large + 1px + min($grid-size, ( 40px - $icon-button-size ) / 2); } .block-editor-link-control__search-results-wrapper { From 15f4d08c52dc65aab874b04deb03c30ff2b079d3 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 6 Feb 2020 14:49:46 -0500 Subject: [PATCH 13/16] Format Library: Return paragraph selection on link edit cancel --- packages/format-library/src/link/index.js | 3 +-- packages/format-library/src/link/inline.js | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 8680d006e41e7..ce3b8e874baf7 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -97,6 +97,7 @@ export const link = { stopAddingLink() { this.setState( { addingLink: false } ); + this.props.onFocus(); } onRemoveFormat() { @@ -112,7 +113,6 @@ export const link = { activeAttributes, value, onChange, - onFocus, } = this.props; return ( @@ -157,7 +157,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 cc5d3636c51f1..46112c731863b 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -31,7 +31,6 @@ function InlineLinkUI( { addingLink, value, onChange, - onFocus, speak, stopAddingLink, } ) { @@ -109,7 +108,6 @@ function InlineLinkUI( { linkValue.opensInNewTab !== nextValue.opensInNewTab && linkValue.url === nextValue.url; if ( ! didToggleSetting ) { - onFocus(); stopAddingLink(); } From d4d8919cbce343b2b326e3503814f540859c60aa Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 6 Feb 2020 16:00:24 -0500 Subject: [PATCH 14/16] Block Editor: LinkControl: Infer absence of value setting as unchecked --- .../block-editor/src/components/link-control/settings-drawer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/settings-drawer.js b/packages/block-editor/src/components/link-control/settings-drawer.js index 0df253d11a1eb..5fc275ef07c66 100644 --- a/packages/block-editor/src/components/link-control/settings-drawer.js +++ b/packages/block-editor/src/components/link-control/settings-drawer.js @@ -38,7 +38,7 @@ const LinkControlSettingsDrawer = ( { key={ setting.id } label={ setting.title } onChange={ handleSettingChange( setting ) } - checked={ value ? value[ setting.id ] : false } + checked={ value ? !! value[ setting.id ] : false } /> ) ); From 8d9fe078ee1cab5d92a7c7ee67b4e1a335181604 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Feb 2020 12:36:53 -0500 Subject: [PATCH 15/16] Format Library: Hold "Open in New Tab" during insert --- .../various/__snapshots__/links.test.js.snap | 8 +++++++- .../specs/editor/various/links.test.js | 17 ++++++++++++++++- packages/format-library/src/link/inline.js | 17 ++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) 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 817be306f4bbf..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 @@ -14,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

" `; diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index 9ad716edca01a..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 diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 46112c731863b..c4f59431d3a13 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'; @@ -50,6 +50,8 @@ function InlineLinkUI( { */ const mountingKey = useMemo( uniqueId, [ addingLink ] ); + const [ nextLinkValue, setNextLinkValue ] = useState(); + const anchorRef = useMemo( () => { const selection = window.getSelection(); @@ -78,9 +80,21 @@ function InlineLinkUI( { const linkValue = { url: activeAttributes.url, opensInNewTab: activeAttributes.target === '_blank', + ...nextLinkValue, }; function onChangeLink( nextValue ) { + nextValue = { + ...nextLinkValue, + ...nextValue, + }; + + setNextLinkValue( nextValue ); + + if ( nextValue.url === undefined ) { + return; + } + const newUrl = prependHTTP( nextValue.url ); const selectedText = getTextContent( slice( value ) ); const format = createLinkFormat( { @@ -109,6 +123,7 @@ function InlineLinkUI( { linkValue.url === nextValue.url; if ( ! didToggleSetting ) { stopAddingLink(); + setNextLinkValue( undefined ); } if ( ! isValidHref( newUrl ) ) { From ae596cdb76ebc36d1408b5aaa96b4a6efc0a5219 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 7 Feb 2020 14:56:16 -0500 Subject: [PATCH 16/16] Format Library: Refactor, comment inline link state flow --- packages/format-library/src/link/inline.js | 37 ++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index c4f59431d3a13..2d9eb18df1578 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -50,6 +50,14 @@ 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( () => { @@ -84,14 +92,30 @@ function InlineLinkUI( { }; 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, }; - setNextLinkValue( nextValue ); + // LinkControl calls `onChange` immediately upon the toggling a setting. + const didToggleSetting = + linkValue.opensInNewTab !== nextValue.opensInNewTab && + linkValue.url === nextValue.url; - if ( nextValue.url === undefined ) { + // 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; } @@ -115,15 +139,10 @@ function InlineLinkUI( { onChange( applyFormat( value, format ) ); } - // LinkControl calls `onChange` immediately upon the toggling of any - // settings, but focus should only be shifted back to the formatted - // segment when the URL is submitted. - const didToggleSetting = - linkValue.opensInNewTab !== nextValue.opensInNewTab && - linkValue.url === nextValue.url; + // Focus should only be shifted back to the formatted segment when the + // URL is submitted. if ( ! didToggleSetting ) { stopAddingLink(); - setNextLinkValue( undefined ); } if ( ! isValidHref( newUrl ) ) {