Skip to content

Commit

Permalink
Block Editor: LinkControl: Always check to retain focus on toggled ed…
Browse files Browse the repository at this point in the history
…iting
  • Loading branch information
aduth committed Apr 9, 2020
1 parent 6614e6e commit 9045b67
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 22 deletions.
66 changes: 44 additions & 22 deletions packages/block-editor/src/components/link-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function LinkControl( {
);
const [ isResolvingLink, setIsResolvingLink ] = useState( false );
const [ errorMessage, setErrorMessage ] = useState( null );
const isEndingEditWithFocus = useRef( false );
const isTogglingEditingWithFocusIntent = useRef( false );

const { fetchSearchSuggestions } = useSelect( ( select ) => {
const { getSettings } = select( 'core/block-editor' );
Expand Down Expand Up @@ -209,7 +209,7 @@ function LinkControl( {
// guaranteed. The link input may continue to be shown if the next value
// is still unassigned after calling `onChange`.
const hadFocusLoss =
isEndingEditWithFocus.current &&
isTogglingEditingWithFocusIntent.current &&
wrapperNode.current &&
! wrapperNode.current.contains( document.activeElement );

Expand All @@ -223,7 +223,7 @@ function LinkControl( {
nextFocusTarget.focus();
}

isEndingEditWithFocus.current = false;
isTogglingEditingWithFocusIntent.current = false;
}, [ isEditingLink ] );

/**
Expand All @@ -241,6 +241,40 @@ function LinkControl( {
};
}, [] );

/**
* Sets editing state and marks that focus may need to be restored after
* the next render, using an optional explicit `setFocus` argument. Normally
* it should not be necessary to call this function, and instead rely on the
* behavior of `setIsEditingLinkWithRetainedFocus`. However, in cases where
* editing state is toggled in response to button clicks, this function
* normalizes browser inconsistencies where focus may not be assigned,
* considering any button click as effectively having focus.
*
* @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
*
* @param {boolean} nextIsEditingLink State toggle value to set.
* @param {boolean} [setFocus=true] Whether focus should be restored.
*/
function setIsEditingLinkWithFocus( nextIsEditingLink, setFocus = true ) {
isTogglingEditingWithFocusIntent.current = setFocus;

setIsEditingLink( nextIsEditingLink );
}

/**
* Sets editing state and marks that focus may need to be restored after
* the next render, if focus was within the wrapper while toggled.
*
* @param {boolean} nextIsEditingLink State toggle value to set.
*/
function setIsEditingLinkWithRetainedFocus( nextIsEditingLink ) {
const setFocus =
!! wrapperNode.current &&
wrapperNode.current.contains( document.activeElement );

setIsEditingLinkWithFocus( nextIsEditingLink, setFocus );
}

/**
* onChange LinkControlSearchInput event handler
*
Expand Down Expand Up @@ -321,18 +355,6 @@ function LinkControl( {
} );
};

/**
* Cancels editing state and marks that focus may need to be restored after
* the next render, if focus was within the wrapper when editing finished.
*/
function stopEditing() {
isEndingEditWithFocus.current =
!! wrapperNode.current &&
wrapperNode.current.contains( document.activeElement );

setIsEditingLink( false );
}

/**
* Determines whether a given value could be a URL. Note this does not
* guarantee the value is a URL only that it looks like it might be one. For
Expand Down Expand Up @@ -386,9 +408,9 @@ function LinkControl( {
// Only set link if request is resolved, otherwise enable edit mode.
if ( newSuggestion ) {
onChange( newSuggestion );
stopEditing();
setIsEditingLinkWithRetainedFocus( false );
} else {
setIsEditingLink( true );
setIsEditingLinkWithRetainedFocus( true );
}
} catch ( error ) {
if ( error && error.isCanceled ) {
Expand All @@ -402,12 +424,12 @@ function LinkControl( {
)
);
setIsResolvingLink( false );
setIsEditingLink( true );
setIsEditingLinkWithRetainedFocus( true );
}
};

const handleSelectSuggestion = ( suggestion, _value = {} ) => {
setIsEditingLink( false );
setIsEditingLinkWithRetainedFocus( false );
onChange( { ..._value, ...suggestion } );
};

Expand Down Expand Up @@ -514,7 +536,7 @@ function LinkControl( {
suggestion={ suggestion }
index={ index }
onClick={ () => {
stopEditing();
setIsEditingLinkWithFocus( false );
onChange( { ...value, ...suggestion } );
} }
isSelected={ index === selectedSuggestion }
Expand Down Expand Up @@ -555,7 +577,7 @@ function LinkControl( {
await handleOnCreate( inputValue );
} else {
handleSelectSuggestion( suggestion, value );
stopEditing();
setIsEditingLinkWithRetainedFocus( false );
}
} }
renderSuggestions={
Expand Down Expand Up @@ -595,7 +617,7 @@ function LinkControl( {

<Button
isSecondary
onClick={ () => setIsEditingLink( true ) }
onClick={ () => setIsEditingLinkWithFocus( true ) }
className="block-editor-link-control__search-item-action"
>
{ __( 'Edit' ) }
Expand Down
22 changes: 22 additions & 0 deletions packages/block-editor/src/components/link-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,28 @@ function eventLoopTick() {

let container = null;

beforeAll( () => {
// This is necessary because the implementation of `@wordpress/dom` focus
// utilities rely on CSSOM layout properties in order to detect an element
// as being focusable. These are not implemented by JSDOM, so are stubbed
// here instead. Ideally, this is either implemented or stubbed in JSDOM,
// mocked globally, or the implementation of focus is revised to not depend
// on these properties.
//
// See: https://github.com/jsdom/jsdom/issues/135
Object.defineProperties( window.HTMLElement.prototype, {
getClientRects: {
get: () => [ new window.DOMRect() ],
},
offsetHeight: {
get: () => 1,
},
offsetWidth: {
get: () => 1,
},
} );
} );

beforeEach( () => {
// setup a DOM element as a render target
container = document.createElement( 'div' );
Expand Down

0 comments on commit 9045b67

Please sign in to comment.