From 432bfe5c52dface0e66c311968aa6771c5805bc9 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Sun, 28 Oct 2018 05:15:03 +0800 Subject: [PATCH] Fix link popover keyboard accessibility (#10983) * Add e2e test covering modifying an existing link using the keyboard * Add state to determine whether the caret is currently within formatting * Show toolbar when the caret is positioned within formatting * Update docs * Change leave to exit --- docs/data/data-core-editor.md | 20 ++++++++++++ .../editor/src/components/block-list/block.js | 5 ++- .../editor/src/components/rich-text/index.js | 12 ++++++- packages/editor/src/store/actions.js | 22 +++++++++++++ packages/editor/src/store/reducer.js | 21 ++++++++++++ packages/editor/src/store/selectors.js | 11 +++++++ packages/editor/src/store/test/actions.js | 18 +++++++++++ packages/editor/src/store/test/reducer.js | 19 +++++++++++ packages/editor/src/store/test/selectors.js | 19 +++++++++++ test/e2e/specs/links.test.js | 32 +++++++++++++++++++ 10 files changed, 177 insertions(+), 2 deletions(-) diff --git a/docs/data/data-core-editor.md b/docs/data/data-core-editor.md index 0fc957c749a0c..6554b8d7535bd 100644 --- a/docs/data/data-core-editor.md +++ b/docs/data/data-core-editor.md @@ -907,6 +907,18 @@ Returns true if the user is typing, or false otherwise. Whether user is typing. +### isCaretWithinFormattedText + +Returns true if the caret is within formatted text, or false otherwise. + +*Parameters* + + * state: Global application state. + +*Returns* + +Whether the caret is within formatted text. + ### getBlockInsertionPoint Returns the insertion point, the index at which the new inserted block would @@ -1623,6 +1635,14 @@ Returns an action object used in signalling that the user has begun to type. Returns an action object used in signalling that the user has stopped typing. +### enterFormattedText + +Returns an action object used in signalling that the caret has entered formatted text. + +### exitFormattedText + +Returns an action object used in signalling that the user caret has exited formatted text. + ### updatePostLock Returns an action object used to lock the editor. diff --git a/packages/editor/src/components/block-list/block.js b/packages/editor/src/components/block-list/block.js index 498c79c3da553..d7493ee0810b6 100644 --- a/packages/editor/src/components/block-list/block.js +++ b/packages/editor/src/components/block-list/block.js @@ -372,6 +372,7 @@ export class BlockListBlock extends Component { isPartOfMultiSelection, isFirstMultiSelected, isTypingWithinBlock, + isCaretWithinFormattedText, isMultiSelecting, hoverArea, isEmptyDefaultBlock, @@ -399,7 +400,7 @@ export class BlockListBlock extends Component { // We render block movers and block settings to keep them tabbale even if hidden const shouldRenderMovers = ! isFocusMode && ( isSelected || hoverArea === 'left' ) && ! showEmptyBlockSideInserter && ! isMultiSelecting && ! isPartOfMultiSelection && ! isTypingWithinBlock; const shouldShowBreadcrumb = ! isFocusMode && isHovered && ! isEmptyDefaultBlock; - const shouldShowContextualToolbar = ! hasFixedToolbar && ! showSideInserter && ( ( isSelected && ! isTypingWithinBlock ) || isFirstMultiSelected ); + const shouldShowContextualToolbar = ! hasFixedToolbar && ! showSideInserter && ( ( isSelected && ( ! isTypingWithinBlock || isCaretWithinFormattedText ) ) || isFirstMultiSelected ); const shouldShowMobileToolbar = shouldAppearSelected; const { error, dragging } = this.state; @@ -588,6 +589,7 @@ const applyWithSelect = withSelect( ( select, { clientId, rootClientId, isLargeV isFirstMultiSelectedBlock, isMultiSelecting, isTyping, + isCaretWithinFormattedText, getBlockIndex, getEditedPostAttribute, getBlockMode, @@ -613,6 +615,7 @@ const applyWithSelect = withSelect( ( select, { clientId, rootClientId, isLargeV // We only care about this prop when the block is selected // Thus to avoid unnecessary rerenders we avoid updating the prop if the block is not selected. isTypingWithinBlock: ( isSelected || isParentOfSelectedBlock ) && isTyping(), + isCaretWithinFormattedText: isCaretWithinFormattedText(), order: getBlockIndex( clientId, rootClientId ), meta: getEditedPostAttribute( 'meta' ), mode: getBlockMode( clientId ), diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 20cf3555bb8bd..49574450ccb8c 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -424,7 +424,13 @@ export class RichText extends Component { return; } - const { start, end } = this.createRecord(); + const { start, end, formats } = this.createRecord(); + + if ( formats[ start ] ) { + this.props.onEnterFormattedText(); + } else { + this.props.onExitFormattedText(); + } if ( start !== this.state.start || end !== this.state.end ) { this.setState( { start, end } ); @@ -962,12 +968,16 @@ const RichTextContainer = compose( [ createUndoLevel, redo, undo, + enterFormattedText, + exitFormattedText, } = dispatch( 'core/editor' ); return { onCreateUndoLevel: createUndoLevel, onRedo: redo, onUndo: undo, + onEnterFormattedText: enterFormattedText, + onExitFormattedText: exitFormattedText, }; } ), withSafeTimeout, diff --git a/packages/editor/src/store/actions.js b/packages/editor/src/store/actions.js index b173122b4e3c9..d074b85ae3d4e 100644 --- a/packages/editor/src/store/actions.js +++ b/packages/editor/src/store/actions.js @@ -535,6 +535,28 @@ export function stopTyping() { }; } +/** + * Returns an action object used in signalling that the caret has entered formatted text. + * + * @return {Object} Action object. + */ +export function enterFormattedText() { + return { + type: 'ENTER_FORMATTED_TEXT', + }; +} + +/** + * Returns an action object used in signalling that the user caret has exited formatted text. + * + * @return {Object} Action object. + */ +export function exitFormattedText() { + return { + type: 'EXIT_FORMATTED_TEXT', + }; +} + /** * Returns an action object used to lock the editor. * diff --git a/packages/editor/src/store/reducer.js b/packages/editor/src/store/reducer.js index 0e9b477eac5d9..e3472344f58c1 100644 --- a/packages/editor/src/store/reducer.js +++ b/packages/editor/src/store/reducer.js @@ -584,6 +584,26 @@ export function isTyping( state = false, action ) { return state; } +/** + * Reducer returning whether the caret is within formatted text. + * + * @param {boolean} state Current state. + * @param {Object} action Dispatched action. + * + * @return {boolean} Updated state. + */ +export function isCaretWithinFormattedText( state = false, action ) { + switch ( action.type ) { + case 'ENTER_FORMATTED_TEXT': + return true; + + case 'EXIT_FORMATTED_TEXT': + return false; + } + + return state; +} + /** * Reducer returning the block selection's state. * @@ -1093,6 +1113,7 @@ export default optimist( combineReducers( { editor, currentPost, isTyping, + isCaretWithinFormattedText, blockSelection, blocksMode, blockListSettings, diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index d82d4edef0cf1..df1ccae61d91d 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -1255,6 +1255,17 @@ export function isTyping( state ) { return state.isTyping; } +/** + * Returns true if the caret is within formatted text, or false otherwise. + * + * @param {Object} state Global application state. + * + * @return {boolean} Whether the caret is within formatted text. + */ +export function isCaretWithinFormattedText( state ) { + return state.isCaretWithinFormattedText; +} + /** * Returns the insertion point, the index at which the new inserted block would * be placed. Defaults to the last index. diff --git a/packages/editor/src/store/test/actions.js b/packages/editor/src/store/test/actions.js index b2ab5cf6c9145..9c776f0cc47e8 100644 --- a/packages/editor/src/store/test/actions.js +++ b/packages/editor/src/store/test/actions.js @@ -5,6 +5,8 @@ import { replaceBlocks, startTyping, stopTyping, + enterFormattedText, + exitFormattedText, fetchReusableBlocks, saveReusableBlock, deleteReusableBlock, @@ -344,6 +346,22 @@ describe( 'actions', () => { } ); } ); + describe( 'enterFormattedText', () => { + it( 'should return the ENTER_FORMATTED_TEXT action', () => { + expect( enterFormattedText() ).toEqual( { + type: 'ENTER_FORMATTED_TEXT', + } ); + } ); + } ); + + describe( 'exitFormattedText', () => { + it( 'should return the EXIT_FORMATTED_TEXT action', () => { + expect( exitFormattedText() ).toEqual( { + type: 'EXIT_FORMATTED_TEXT', + } ); + } ); + } ); + describe( 'fetchReusableBlocks', () => { it( 'should return the FETCH_REUSABLE_BLOCKS action', () => { expect( fetchReusableBlocks() ).toEqual( { diff --git a/packages/editor/src/store/test/reducer.js b/packages/editor/src/store/test/reducer.js index 9ebca4c39c10c..c0462df96f411 100644 --- a/packages/editor/src/store/test/reducer.js +++ b/packages/editor/src/store/test/reducer.js @@ -25,6 +25,7 @@ import { editor, currentPost, isTyping, + isCaretWithinFormattedText, blockSelection, preferences, saving, @@ -1315,6 +1316,24 @@ describe( 'state', () => { } ); } ); + describe( 'isCaretWithinFormattedText()', () => { + it( 'should set the flag to true', () => { + const state = isCaretWithinFormattedText( false, { + type: 'ENTER_FORMATTED_TEXT', + } ); + + expect( state ).toBe( true ); + } ); + + it( 'should set the flag to false', () => { + const state = isCaretWithinFormattedText( true, { + type: 'EXIT_FORMATTED_TEXT', + } ); + + expect( state ).toBe( false ); + } ); + } ); + describe( 'blockSelection()', () => { it( 'should return with block clientId as selected', () => { const state = blockSelection( undefined, { diff --git a/packages/editor/src/store/test/selectors.js b/packages/editor/src/store/test/selectors.js index 108f8dcf69e01..bd348c1357ead 100644 --- a/packages/editor/src/store/test/selectors.js +++ b/packages/editor/src/store/test/selectors.js @@ -82,6 +82,7 @@ const { isFirstMultiSelectedBlock, getBlockMode, isTyping, + isCaretWithinFormattedText, getBlockInsertionPoint, isBlockInsertionPointVisible, isSavingPost, @@ -2719,6 +2720,24 @@ describe( 'selectors', () => { } ); } ); + describe( 'isCaretWithinFormattedText', () => { + it( 'returns true if the isCaretWithinFormattedText state is also true', () => { + const state = { + isCaretWithinFormattedText: true, + }; + + expect( isCaretWithinFormattedText( state ) ).toBe( true ); + } ); + + it( 'returns false if the isCaretWithinFormattedText state is also false', () => { + const state = { + isCaretWithinFormattedText: false, + }; + + expect( isCaretWithinFormattedText( state ) ).toBe( false ); + } ); + } ); + describe( 'isSelectionEnabled', () => { it( 'should return true if selection is enable', () => { const state = { diff --git a/test/e2e/specs/links.test.js b/test/e2e/specs/links.test.js index 23546f8c72045..850ad3d07afc9 100644 --- a/test/e2e/specs/links.test.js +++ b/test/e2e/specs/links.test.js @@ -388,4 +388,36 @@ describe( 'Links', () => { await page.keyboard.press( 'Escape' ); expect( await page.$( '.editor-url-popover' ) ).toBeNull(); } ); + + it( 'can be modified using the keyboard once a link has been set', async () => { + const URL = 'https://wordpress.org/gutenberg'; + + // Create a block with some text and format it as a link. + await clickBlockAppender(); + await page.keyboard.type( 'This is Gutenberg' ); + await pressWithModifier( SELECT_WORD_MODIFIER_KEYS, 'ArrowLeft' ); + await pressWithModifier( META_KEY, 'K' ); + await waitForAutoFocus(); + await page.keyboard.type( URL ); + await page.keyboard.press( 'Enter' ); + + // 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.$( '.editor-url-popover' ) ).toBeNull(); + + // Move the caret back into the link text and the link popover + // should be displayed. + await page.keyboard.press( 'ArrowLeft' ); + expect( await page.$( '.editor-url-popover' ) ).not.toBeNull(); + + // Press Cmd+K to edit the link and the url-input should become + // focused with the value previously inserted. + await pressWithModifier( META_KEY, 'K' ); + await waitForAutoFocus(); + const activeElementParentClasses = await page.evaluate( () => Object.values( document.activeElement.parentElement.classList ) ); + expect( activeElementParentClasses ).toContain( 'editor-url-input' ); + const activeElementValue = await page.evaluate( () => document.activeElement.value ); + expect( activeElementValue ).toBe( URL ); + } ); } );