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.

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

Move linkUI to own custom appender in navigation block #68109

Open
wants to merge 14 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export default function BlockListAppender( {
data-block
>
{ CustomAppender ? (
<CustomAppender />
<CustomAppender rootClientId={ rootClientId } />
) : (
<DefaultAppender rootClientId={ rootClientId } />
) }
Expand Down
67 changes: 67 additions & 0 deletions packages/block-library/src/navigation-link/appender.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* WordPress dependencies
*/
import { useDispatch } from '@wordpress/data';
import { useState, useRef } from '@wordpress/element';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { Icon, plus } from '@wordpress/icons';
import { Button } from '@wordpress/components';
import { createBlock } from '@wordpress/blocks';

/**
* Internal dependencies
*/
import { LinkUI } from './link-ui';
import { updateAttributes } from './update-attributes';

export default function NavigationLinkAppender( { rootClientId } ) {
const { insertBlock } = useDispatch( blockEditorStore );
const [ isLinkOpen, setIsLinkOpen ] = useState( false );
const [ popoverAnchor, setPopoverAnchor ] = useState( null );
const linkUIref = useRef();

const createNewLink = ( attributes ) => {
const block = createBlock( 'core/navigation-link', attributes );

insertBlock( block, undefined, rootClientId );
};

return (
<>
<Button
__next40pxDefaultSize
ref={ setPopoverAnchor }
className="block-editor-button-block-appender"
onClick={ () => setIsLinkOpen( ( state ) => ! state ) }
aria-haspopup
aria-expanded={ isLinkOpen }
label="Add block"
showTooltip
>
<Icon icon={ plus } />
</Button>
{ isLinkOpen && (
<LinkUI
ref={ linkUIref }
clientId={ rootClientId }
link={ {
url: undefined,
label: undefined,
id: undefined,
kind: undefined,
type: undefined,
opensInNewTab: false,
} }
onClose={ () => {
setIsLinkOpen( false );
} }
onChange={ ( updatedValue ) => {
setIsLinkOpen( false );
updateAttributes( updatedValue, createNewLink );
} }
anchor={ popoverAnchor }
/>
) }
</>
);
}
79 changes: 41 additions & 38 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { __unstableStripHTML as stripHTML } from '@wordpress/dom';
import { decodeEntities } from '@wordpress/html-entities';
import { link as linkIcon, addSubmenu } from '@wordpress/icons';
import { store as coreStore } from '@wordpress/core-data';
import { useMergeRefs, usePrevious } from '@wordpress/compose';
import { useMergeRefs } from '@wordpress/compose';

/**
* Internal dependencies
Expand All @@ -42,8 +42,6 @@ import { LinkUI } from './link-ui';
import { updateAttributes } from './update-attributes';
import { getColors } from '../navigation/edit/utils';

const DEFAULT_BLOCK = { name: 'core/navigation-link' };

/**
* A React hook to determine if it's dragging within the target element.
*
Expand Down Expand Up @@ -250,10 +248,20 @@ export default function NavigationLinkEdit( {
selectBlock,
selectPreviousBlock,
} = useDispatch( blockEditorStore );
// Have the link editing ui open on mount when lacking a url and selected.
const [ isLinkOpen, setIsLinkOpen ] = useState( isSelected && ! url );

// If the link isSelected when mounted, it was just added via an appender
const isNewLink = isSelected;
const isLabelURLLike =
url && isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label );

const [ isLinkOpen, setIsLinkOpen ] = useState(
isNewLink && ! isLabelURLLike
);

// If the link ui was opened because it was just added, we need to return focus to the block instead of
// the appender when it is closed.
const focusBlockOnLinkCloseRef = useRef( isLinkOpen );
// Store what element opened the popover, so we know where to return focus to (toolbar button vs navigation link text)
const [ openedBy, setOpenedBy ] = useState( null );
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ popoverAnchor, setPopoverAnchor ] = useState( null );
Expand All @@ -262,7 +270,14 @@ export default function NavigationLinkEdit( {
const itemLabelPlaceholder = __( 'Add label…' );
const ref = useRef();
const linkUIref = useRef();
const prevUrl = usePrevious( url );

useEffect( () => {
// If the link was just added and label is url-like, focus and select the label text.
if ( isNewLink && isLabelURLLike ) {
// Focus and select the label text.
selectLabelText();
}
}, [] );
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect you want the dependencies to rerun this like it was before.

Suggested change
}, [] );
}, [ isLabelURLLike, isNewLink ] );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only want this to run on mount.


// Change the label using inspector causes rich text to change focus on firefox.
// This is a workaround to keep the focus on the label field when label filed is focused we don't render the rich text.
Expand Down Expand Up @@ -301,6 +316,7 @@ export default function NavigationLinkEdit( {
},
[ clientId, maxNestingLevel ]
);

const { getBlocks } = useSelect( blockEditorStore );

/**
Expand Down Expand Up @@ -330,21 +346,6 @@ export default function NavigationLinkEdit( {
}
}, [ hasChildren ] );

// If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text.
useEffect( () => {
// We only want to do this when the URL has gone from nothing to a new URL AND the label looks like a URL
if (
! prevUrl &&
url &&
isLinkOpen &&
isURL( prependHTTP( label ) ) &&
/^.+\.[a-z]+/.test( label )
) {
// Focus and select the label text.
selectLabelText();
}
}, [ prevUrl, url, isLinkOpen, label ] );

/**
* Focus the Link label text and select it.
*/
Expand Down Expand Up @@ -398,7 +399,6 @@ export default function NavigationLinkEdit( {
// If this link is a child of a parent submenu item, the parent submenu item event will also open, closing this popover
event.stopPropagation();
setIsLinkOpen( true );
setOpenedBy( ref.current );
}
}

Expand Down Expand Up @@ -428,7 +428,6 @@ export default function NavigationLinkEdit( {
className: 'remove-outline', // Remove the outline from the inner blocks container.
},
{
defaultBlock: DEFAULT_BLOCK,
directInsert: true,
renderAppender: false,
}
Expand All @@ -437,7 +436,6 @@ export default function NavigationLinkEdit( {
if ( ! url || isInvalid || isDraft ) {
blockProps.onClick = () => {
setIsLinkOpen( true );
setOpenedBy( ref.current );
};
}

Expand All @@ -464,9 +462,8 @@ export default function NavigationLinkEdit( {
icon={ linkIcon }
title={ __( 'Link' ) }
shortcut={ displayShortcut.primary( 'k' ) }
onClick={ ( event ) => {
onClick={ () => {
setIsLinkOpen( true );
setOpenedBy( event.currentTarget );
} }
/>
{ ! isAtMaxNesting && (
Expand Down Expand Up @@ -569,8 +566,12 @@ export default function NavigationLinkEdit( {
clientId={ clientId }
link={ attributes }
onClose={ () => {
// If there is no link then remove the auto-inserted block.
// If there is no link then remove the block.
// This avoids empty blocks which can provided a poor UX.
// This should not happen often. The main route is through adding a custom link block.
// An alternative to this would be to open the custom link block
// directly to the edit link popover with text and url fields, rather than the LinkUI
// where you can select a block as well.
if ( ! url ) {
// Fixes https://github.com/WordPress/gutenberg/issues/61361
// There's a chance we're closing due to the user selecting the browse all button.
Expand All @@ -591,17 +592,19 @@ export default function NavigationLinkEdit( {
return;
}

setIsLinkOpen( false );
if ( openedBy ) {
openedBy.focus();
setOpenedBy( null );
} else if ( ref.current ) {
// select the ref when adding a new link
ref.current.focus();
} else {
// Fallback
selectPreviousBlock( clientId, true );
// If the link ui was opened due to the block being added,
// we need to return focus to the block instead of the appender.
if (
focusBlockOnLinkCloseRef.current &&
linkUIref.current.contains(
window.document.activeElement
)
) {
ref.current?.focus();
}

setIsLinkOpen( false );
focusBlockOnLinkCloseRef.current = false;
} }
anchor={ popoverAnchor }
onRemove={ removeLink }
Expand Down
19 changes: 3 additions & 16 deletions packages/block-library/src/navigation-link/link-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
import { __, sprintf, isRTL } from '@wordpress/i18n';
import {
LinkControl,
store as blockEditorStore,
privateApis as blockEditorPrivateApis,
} from '@wordpress/block-editor';
import {
Expand All @@ -27,7 +26,7 @@ import {
useResourcePermissions,
} from '@wordpress/core-data';
import { decodeEntities } from '@wordpress/html-entities';
import { useSelect, useDispatch } from '@wordpress/data';
import { useDispatch } from '@wordpress/data';
import { chevronLeftSmall, chevronRightSmall, plus } from '@wordpress/icons';
import { useInstanceId, useFocusOnMount } from '@wordpress/compose';

Expand Down Expand Up @@ -79,17 +78,6 @@ export function getSuggestionsQuery( type, kind ) {
}

function LinkUIBlockInserter( { clientId, onBack, onSelectBlock } ) {
const { rootBlockClientId } = useSelect(
( select ) => {
const { getBlockRootClientId } = select( blockEditorStore );

return {
rootBlockClientId: getBlockRootClientId( clientId ),
};
},
[ clientId ]
);

const focusOnMountRef = useFocusOnMount( 'firstElement' );

const dialogTitleId = useInstanceId(
Expand Down Expand Up @@ -134,9 +122,8 @@ function LinkUIBlockInserter( { clientId, onBack, onSelectBlock } ) {
</Button>

<QuickInserter
rootClientId={ rootBlockClientId }
clientId={ clientId }
isAppender={ false }
rootClientId={ clientId }
isAppender
prioritizePatterns={ false }
selectBlockOnInsert
hasSearch={ false }
Expand Down
24 changes: 5 additions & 19 deletions packages/block-library/src/navigation-submenu/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { displayShortcut, isKeyboardEvent } from '@wordpress/keycodes';
import { __ } from '@wordpress/i18n';
import {
BlockControls,
InnerBlocks,
useInnerBlocksProps,
InspectorControls,
RichText,
Expand All @@ -39,6 +38,7 @@ import { useMergeRefs, usePrevious } from '@wordpress/compose';
*/
import { ItemSubmenuIcon } from './icons';
import { LinkUI } from '../navigation-link/link-ui';
import NavigationLinkAppender from '../navigation-link/appender';
import { updateAttributes } from '../navigation-link/update-attributes';
import {
getColors,
Expand Down Expand Up @@ -138,14 +138,9 @@ export default function NavigationSubmenuEdit( {

const { showSubmenuIcon, maxNestingLevel, openSubmenusOnClick } = context;

const {
__unstableMarkNextChangeAsNotPersistent,
replaceBlock,
selectBlock,
} = useDispatch( blockEditorStore );
const { __unstableMarkNextChangeAsNotPersistent, replaceBlock } =
useDispatch( blockEditorStore );
const [ isLinkOpen, setIsLinkOpen ] = useState( false );
// Store what element opened the popover, so we know where to return focus to (toolbar button vs navigation link text)
const [ openedBy, setOpenedBy ] = useState( null );
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ popoverAnchor, setPopoverAnchor ] = useState( null );
Expand Down Expand Up @@ -277,7 +272,6 @@ export default function NavigationSubmenuEdit( {
// If we don't stop propogation, this event bubbles up to the parent submenu item
event.stopPropagation();
setIsLinkOpen( true );
setOpenedBy( ref.current );
}
}

Expand Down Expand Up @@ -330,7 +324,7 @@ export default function NavigationSubmenuEdit( {
! selectedBlockHasChildren ) ||
// Show the appender while dragging to allow inserting element between item and the appender.
hasChildren
? InnerBlocks.ButtonBlockAppender
? NavigationLinkAppender
: false,
} );

Expand Down Expand Up @@ -364,9 +358,8 @@ export default function NavigationSubmenuEdit( {
icon={ linkIcon }
title={ __( 'Link' ) }
shortcut={ displayShortcut.primary( 'k' ) }
onClick={ ( event ) => {
onClick={ () => {
setIsLinkOpen( true );
setOpenedBy( event.currentTarget );
} }
/>
) }
Expand Down Expand Up @@ -517,7 +510,6 @@ export default function NavigationSubmenuEdit( {
onClick={ () => {
if ( ! openSubmenusOnClick && ! url ) {
setIsLinkOpen( true );
setOpenedBy( ref.current );
}
} }
/>
Expand All @@ -532,12 +524,6 @@ export default function NavigationSubmenuEdit( {
link={ attributes }
onClose={ () => {
setIsLinkOpen( false );
if ( openedBy ) {
openedBy.focus();
setOpenedBy( null );
} else {
selectBlock( clientId );
}
} }
anchor={ popoverAnchor }
onRemove={ () => {
Expand Down
Loading
Loading