Skip to content

Commit

Permalink
Fix link popover keyboard accessibility (#10983)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
talldan authored and tofumatt committed Oct 27, 2018
1 parent aec9408 commit 432bfe5
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 2 deletions.
20 changes: 20 additions & 0 deletions docs/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion packages/editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ export class BlockListBlock extends Component {
isPartOfMultiSelection,
isFirstMultiSelected,
isTypingWithinBlock,
isCaretWithinFormattedText,
isMultiSelecting,
hoverArea,
isEmptyDefaultBlock,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -588,6 +589,7 @@ const applyWithSelect = withSelect( ( select, { clientId, rootClientId, isLargeV
isFirstMultiSelectedBlock,
isMultiSelecting,
isTyping,
isCaretWithinFormattedText,
getBlockIndex,
getEditedPostAttribute,
getBlockMode,
Expand All @@ -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 ),
Expand Down
12 changes: 11 additions & 1 deletion packages/editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } );
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions packages/editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
21 changes: 21 additions & 0 deletions packages/editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -1093,6 +1113,7 @@ export default optimist( combineReducers( {
editor,
currentPost,
isTyping,
isCaretWithinFormattedText,
blockSelection,
blocksMode,
blockListSettings,
Expand Down
11 changes: 11 additions & 0 deletions packages/editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions packages/editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
replaceBlocks,
startTyping,
stopTyping,
enterFormattedText,
exitFormattedText,
fetchReusableBlocks,
saveReusableBlock,
deleteReusableBlock,
Expand Down Expand Up @@ -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( {
Expand Down
19 changes: 19 additions & 0 deletions packages/editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
editor,
currentPost,
isTyping,
isCaretWithinFormattedText,
blockSelection,
preferences,
saving,
Expand Down Expand Up @@ -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, {
Expand Down
19 changes: 19 additions & 0 deletions packages/editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ const {
isFirstMultiSelectedBlock,
getBlockMode,
isTyping,
isCaretWithinFormattedText,
getBlockInsertionPoint,
isBlockInsertionPointVisible,
isSavingPost,
Expand Down Expand Up @@ -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 = {
Expand Down
32 changes: 32 additions & 0 deletions test/e2e/specs/links.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
} );

0 comments on commit 432bfe5

Please sign in to comment.