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

Link UI incorrectly anchored to "last" link within rich text contenteditable #58280

Closed
getdave opened this issue Jan 25, 2024 · 3 comments · Fixed by #58282
Closed

Link UI incorrectly anchored to "last" link within rich text contenteditable #58280

getdave opened this issue Jan 25, 2024 · 3 comments · Fixed by #58282
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Jan 25, 2024

If you have multiple links within a block of rich text then the Link UI will be "anchor" to the final first link you clicked in the block.

So if you click the first link in the block and then click on other links, the popover will continue to anchor to the first link. Similarly if you clicked the last link first, it would continue to anchor to that link.

It should be anchored to the link that triggers it. I suspect this is due to the way in which we recently changed the usage of useAnchor to cache the first position.

// As you change the link by interacting with the Link UI
// the return value of document.getSelection jumps to the field you're editing,
// not the highlighted text. Given that useAnchor uses document.getSelection,
// it will return null, since it can't find the <mark> element within the Link UI.
// This caches the last truthy value of the selection anchor reference.
// This ensures the Popover is positioned correctly on initial submission of the link.
const cachedRect = useCachedTruthy( popoverAnchor.getBoundingClientRect() );
popoverAnchor.getBoundingClientRect = () => cachedRect;

Screen.Capture.on.2024-01-25.at.20-41-55.mp4
@getdave getdave added [Type] Bug An existing feature does not function as intended [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Jan 25, 2024
@getdave
Copy link
Contributor Author

getdave commented Jan 25, 2024

cc @richtabor @jeryj @scruffian

@getdave
Copy link
Contributor Author

getdave commented Jan 25, 2024

I confirmed that removing theses lines...

https://github.com/WordPress/gutenberg/blob/trunk/packages/format-library/src/link/inline.js#L208-L215

...will "fix" the problem. But now if you create a new link and submit you'll see the popover is incorrectly positioned.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jan 25, 2024
@jeryj
Copy link
Contributor

jeryj commented Jan 25, 2024

The same problem lines also cause an issue where the popover is "fixed" on the page, so it doesn't stay attached to the link, but scrolls away from it.

But now if you create a new link and submit you'll see the popover is incorrectly positioned.

I think the incorrectly positioned new link issue has to do with the popover anchor changing from the paragraph to the link, but the popover anchor doesn't update to be anchored to the new element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants