Don't show Link preview when no selection #58771
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
draganescu
added
[Type] Bug
|
Size Change: +22 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
getdave
reviewed
Feb 7, 2024
| // close the Link popover if there is no active selection | ||
| // after the link was added - this can happen if the user | ||
| // adds a link without any text selected | ||
| if ( addingLink && ! value.activeFormats ) { |
There was a problem hiding this comment.
!value.activeFormats would mean there are no active formats of any kind (including bold, italitc...etc). Is this intended? If not then the isActive prop passed to this component might serve equally well:
There was a problem hiding this comment.
Having no activeFormats entry - as opposed to an empty [] list - in value is specific to when the selection is collapsed after the format is applied to the inserted link.
This is why it feels this fix is exploiting some other problem :)
draganescu
requested review from
ellatrix,
fluiddot and
dcalhoun
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
When I insert the link my cursor doesn't get returned to the canvas: Feb-08-2024.14-41-14.mp4 |
draganescu
force-pushed
the
fix/empty-link-ui
branch
from
6f03367
to
7411e44
Compare
|
Flaky tests detected in 7411e4463ef1d4f7da3e9b7df0f9f3f4e3fb885f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7835487221
|
draganescu
force-pushed
the
fix/empty-link-ui
branch
from
7411e44
to
e51005f
Compare
…r the 1st render with no active edit
|
I like the idea here. |
scruffian
approved these changes
Feb 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Closes #58415
Why?
Bug fix
How?
Close the link ui preview if after the link is added there is no selection, hence no active format exists.
Alternatively another way would be to place the caret at the edge of the new link but inside the format. Although this seems like a better idea, I like the UX here better. I am also not sure there is a bug with caret placement, since there is no selection to count into.
My proposal is that if a link is added without any selection it should be an event that leaves the focus inline to allow for further text entry without the need to close distracting UI. So even if an alternate way is implemented I'd still favour the resulting UX of this PR.
Testing Instructions
Screenshots or screencast
close-link-preview-no-selection.mp4