Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix button block focus trap after a URL has been added #34314

Merged
merged 4 commits into from
Aug 27, 2021
Merged

Fix button block focus trap after a URL has been added #34314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a4630c
Rework button block link UI to match RichText format implementation
talldan Aug 26, 2021
8465a8a
Refine some more, determine visibility by selection and url state
talldan Aug 26, 2021
22ba1b4
Add e2e test
talldan Aug 26, 2021
41590fb
Also focus rich text when unlinking using a keyboard shortcut
talldan Aug 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 44 additions & 17 deletions packages/block-library/src/button/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useCallback, useState, useRef } from '@wordpress/element';
import { useCallback, useEffect, useState, useRef } from '@wordpress/element';
import {
Button,
ButtonGroup,
Expand Down Expand Up @@ -73,27 +73,42 @@ function URLPicker( {
opensInNewTab,
onToggleOpenInNewTab,
anchorRef,
richTextRef,
} ) {
const [ isURLPickerOpen, setIsURLPickerOpen ] = useState( false );
const urlIsSet = !! url;
const urlIsSetandSelected = urlIsSet && isSelected;
const openLinkControl = () => {
setIsURLPickerOpen( true );
return false; // prevents default behaviour for event
const [ isEditingURL, setIsEditingURL ] = useState( false );
const isURLSet = !! url;

const startEditing = ( event ) => {
event.preventDefault();
setIsEditingURL( true );
};
const unlinkButton = () => {

const unlink = () => {
setAttributes( {
url: undefined,
linkTarget: undefined,
rel: undefined,
} );
setIsURLPickerOpen( false );
setIsEditingURL( false );
};
const linkControl = ( isURLPickerOpen || urlIsSetandSelected ) && (

useEffect( () => {
if ( ! isSelected ) {
setIsEditingURL( false );
}
}, [ isSelected ] );

const isLinkControlVisible = isSelected && ( isEditingURL || isURLSet );

const linkControl = isLinkControlVisible && (
<Popover
position="bottom center"
onClose={ () => setIsURLPickerOpen( false ) }
onClose={ () => {
setIsEditingURL( false );
richTextRef.current?.focus();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the original caret selection in the rich text would be restored rather than just a focus.

@ellatrix would you know the best way to do that?

} }
anchorRef={ anchorRef?.current }
focusOnMount={ isEditingURL ? 'firstElement' : false }
>
<LinkControl
className="wp-block-navigation-link__inline-link-input"
Expand All @@ -108,28 +123,34 @@ function URLPicker( {
onToggleOpenInNewTab( newOpensInNewTab );
}
} }
onRemove={ () => {
unlink();
richTextRef.current?.focus();
} }
forceIsEditingLink={ isEditingURL }
/>
</Popover>
);

return (
<>
<BlockControls group="block">
{ ! urlIsSet && (
{ ! isURLSet && (
<ToolbarButton
name="link"
icon={ link }
title={ __( 'Link' ) }
shortcut={ displayShortcut.primary( 'k' ) }
onClick={ openLinkControl }
onClick={ startEditing }
/>
) }
{ urlIsSetandSelected && (
{ isURLSet && (
<ToolbarButton
name="link"
icon={ linkOff }
title={ __( 'Unlink' ) }
shortcut={ displayShortcut.primaryShift( 'k' ) }
onClick={ unlinkButton }
onClick={ unlink }
isActive={ true }
/>
) }
Expand All @@ -138,8 +159,11 @@ function URLPicker( {
<KeyboardShortcuts
bindGlobal
shortcuts={ {
[ rawShortcut.primary( 'k' ) ]: openLinkControl,
[ rawShortcut.primaryShift( 'k' ) ]: unlinkButton,
[ rawShortcut.primary( 'k' ) ]: startEditing,
[ rawShortcut.primaryShift( 'k' ) ]: () => {
unlink();
richTextRef.current?.focus();
},
} }
/>
) }
Expand Down Expand Up @@ -201,6 +225,7 @@ function ButtonEdit( props ) {
const colorProps = useColorProps( attributes );
const spacingProps = useSpacingProps( attributes );
const ref = useRef();
const richTextRef = useRef();
const blockProps = useBlockProps( { ref } );

return (
Expand All @@ -213,6 +238,7 @@ function ButtonEdit( props ) {
} ) }
>
<RichText
ref={ richTextRef }
aria-label={ __( 'Button text' ) }
placeholder={ placeholder || __( 'Add text…' ) }
value={ text }
Expand Down Expand Up @@ -252,6 +278,7 @@ function ButtonEdit( props ) {
opensInNewTab={ linkTarget === '_blank' }
onToggleOpenInNewTab={ onToggleOpenInNewTab }
anchorRef={ ref }
richTextRef={ richTextRef }
/>
<InspectorControls>
<WidthPanel
Expand Down
30 changes: 30 additions & 0 deletions packages/e2e-tests/specs/editor/blocks/buttons.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,36 @@ describe( 'Buttons', () => {
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'moves focus from the link editor back to the button when escape is pressed after the URL has been submitted', async () => {
// Regression: https://github.com/WordPress/gutenberg/issues/34307
await insertBlock( 'Buttons' );
await pressKeyWithModifier( 'primary', 'k' );
await page.waitForFunction(
() => !! document.activeElement.closest( '.block-editor-url-input' )
);
await page.keyboard.type( 'https://example.com' );
await page.keyboard.press( 'Enter' );
await page.waitForFunction(
() =>
document.activeElement ===
document.querySelector(
'.block-editor-link-control a[href="https://example.com"]'
)
);
await page.keyboard.press( 'Escape' );

// Focus should move from the link control to the button block's text.
await page.waitForFunction(
() =>
document.activeElement ===
document.querySelector( '[aria-label="Button text"]' )
);

// The link control should still be visible when a URL is set.
const linkControl = await page.$( '.block-editor-link-control' );
expect( linkControl ).toBeTruthy();
} );

it( 'can jump to the link editor using the keyboard shortcut', async () => {
await insertBlock( 'Buttons' );
await page.keyboard.type( 'WordPress' );
Expand Down