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

Keep Link UI open upon initial link creation when used in RichText #57726

Merged
merged 17 commits into from
Jan 24, 2024
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Correctly position Popover on link creation and avoid focus shifts du…
…e to forced remounts.
  • Loading branch information
getdave authored and draganescu committed Jan 23, 2024
commit 2b055be236d3ec3fd02a5624c2cca6d883263f60
35 changes: 17 additions & 18 deletions packages/format-library/src/link/inline.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { useSelect } from '@wordpress/data';
*/
import { createLinkFormat, isValidHref, getFormatBoundary } from './utils';
import { link as settings } from './index';
import useLinkInstanceKey from './use-link-instance-key';

const LINK_SETTINGS = [
...LinkControl.DEFAULT_LINK_SETTINGS,
Expand Down Expand Up @@ -90,12 +89,9 @@ function InlineLinkUI( {
}

function onChangeLink( nextValue ) {
// LinkControl calls `onChange` immediately upon the toggling a setting.
// Before merging the next value with the current link value, check if
// the setting was toggled.
const didToggleSetting =
linkValue.opensInNewTab !== nextValue.opensInNewTab &&
nextValue.url === undefined;
const hasLink = linkValue?.url;
const isNewLink = ! hasLink;

// Merge the next value with the current link value.
nextValue = {
...linkValue,
Expand Down Expand Up @@ -181,12 +177,22 @@ function InlineLinkUI( {
newValue.start = newValue.end;
getdave marked this conversation as resolved.
Show resolved Hide resolved

onChange( newValue );

// Force update the anchor reference to ensure the Popover is positioned correctly/
// Incorrect positioning can happen on the initial creation of a link because the
// anchor changes from a rich text selection to a link format element (e.g. <a>).
// however the Popover is not repositioned to account for this change because the
// dependencies of `useAnchor` do not change.
popoverAnchor?.update();
}

// Focus should only be shifted back to the formatted segment when the
// URL is submitted.
if ( ! didToggleSetting ) {
stopAddingLink();
// Focus should only be returned to the rich text on submit if this link is not
// being created for the first time. If it is then focus should remain within the
// Link UI because it should remain open for the user to modify the link they have
// just created.
if ( ! isNewLink ) {
const returnFocusToRichText = true;
stopAddingLink( returnFocusToRichText );
}

if ( ! isValidHref( newUrl ) ) {
Expand All @@ -208,12 +214,6 @@ function InlineLinkUI( {
settings,
} );

// Generate a string based key that is unique to this anchor reference.
// This is used to force re-mount the LinkControl component to avoid
// potential stale state bugs caused by the component not being remounted
// See https://github.com/WordPress/gutenberg/pull/34742.
const forceRemountKey = useLinkInstanceKey( popoverAnchor );

// Focus should only be moved into the Popover when the Link is being created or edited.
// When the Link is in "preview" mode focus should remain on the rich text because at
// this point the Link dialog is informational only and thus the user should be able to
Expand Down Expand Up @@ -258,7 +258,6 @@ function InlineLinkUI( {
shift
>
<LinkControl
key={ forceRemountKey }
value={ linkValue }
onChange={ onChangeLink }
onRemove={ removeLink }
Expand Down