-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix Link UI popover anchor in rich text #58282
Conversation
Size Change: +2 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
if ( ! isActive ) { | ||
popoverAnchor.getBoundingClientRect = () => cachedRect; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If isActive
is true then there is an existing link and thus we should allow popoverAnchor
(derived from useAnchor
) to use the default implementation which will seek the <a>
in the DOM and anchor to that.
Otherwise if we're creating a link then there is no <a>
until the link is submitted. Therefore we need to cache the anchor value of the original text selection that the user selected to be linked.
Previously we just cached the selection value always, which wasn't correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes it for existing links, but for new links, the popover will have the same scroll issue since its position is cached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm that this fixes the problem. Thanks for the fix.
…zy-hydration * origin/trunk: (47 commits) Interactivity API: Break up long hydration task in interactivity init (#58227) Add supports.interactivity to Query block (#58316) Font Library: Make notices more consistent (#58180) Fix Global styles text settings bleeding into placeholder component (#58303) Fix the position and size of the Options menu, (#57515) DataViews: Fix safari grid row height issue (#58302) Try a fix (#58282) Navigation Submenu Block: Make block name affect list view (#58296) Apply custom scroll style to fixed header block toolbar (#57444) Add support for transform and letter spacing controls in Global Styles > Typography > Elements (#58142) DataViews: Fix table view cell wrapper and BlockPreviews (#58062) Workflows: Add 'Technical Prototype' to the type-related labels list (#58163) Block Editor: Optimize the 'useBlockDisplayTitle' hook (#58250) Remove noahtallen from .wp-env codeowners (#58283) Global styles revisions: fix is-selected rules from affecting other areas of the editor (#58228) Try: Disable text selection for post content placeholder block. (#58169) Remove `template-only` mode from editor and edit-post packages (#57700) Refactored download/upload logic to support font faces with multiple src assets (#58216) Components: Expand theming support in COLORS (#58097) Implementing new UX for invoking rich text Link UI (#57986) ...
What?
Fixes bug where Link UI Popover anchor becomes divorced from the link that "owns" it (see Issue).
Closes #58280
Why?
The Link UI should always be anchored to the link that was clicked/activated.
How?
Ensures that the anchor position is allowed to be dynamically calculated for existing links where there is a
<a>
tag in the contenteditable element.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2024-01-26.at.08-50-26.mp4