Enhance link control UI with rich URL previews #31464
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
getdave
added
the
[Feature] Link Editing
|
Size Change: +1.31 kB (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
3 tasks
getdave
force-pushed
the
try/enhance-link-control-with-url-data
branch
from
4b6bd40 to
dbf8961
Compare
|
cc @javierarce as you don't seem to be a member of this repo. |
getdave
force-pushed
the
try/enhance-link-control-with-url-data
branch
from
2f03581 to
dff46e3
Compare
|
This works really nice! Some things I've noticed:
I also saw some a couple of bugs: The height of the popover should hug the content:
The icon is on top of the text (although this could be solved if we get rid of the icon)
|
getdave
requested review from
ellatrix,
spacedmonkey and
TimothyBJacobs
as code owners
@javierarce Thanks! 👍 I've made changes as per your feedback and updated the screenshot in the PR description. Let me know how this looks to you now. Some notes from me:
Now capped to 2 lines using our new
I've constrained the image to 140px which is as per the Figma. I will say I think it's too small as most images now cannot fill the space and we see a lot of gray background.
👍 Added to the background color.
Removed.
I wasn't sure what you mean there.
The icon is currently 16x16. Why? Because the favicon we get back from most remote sites is 32x32 and low quality. By scaling down by 50% it gives the illusion of better quality. As it's smaller I've made it align with the top of the text as that seemed to look better. Do you agree? @hellofromtonya perhaps in future we can look to parse an |
getdave
force-pushed
the
try/enhance-link-control-with-url-data
branch
2 times, most recently
from
2207119 to
a07758d
Compare
getdave
requested review from
adamsilverstein,
ajitbohra,
atimmer,
cameronvoell,
chrisvanpatten,
dmsnell,
guarani,
gziolo and
kevin940726
as code owners
getdave
force-pushed
the
try/enhance-link-control-with-url-data
branch
from
0b850b8 to
3f3d200
Compare
|
Ok, I think
Once merged we can followup with the addition of |
getdave
force-pushed
the
try/enhance-link-control-with-url-data
branch
from
3f3d200 to
ebb9346
Compare
10 tasks
|
I think this looks and works nice, but I think we should address the hiding of the placeholders at some point, that would make this look really great. |
This was referenced Jun 9, 2021
kevin940726
approved these changes
Jun 9, 2021
packages/block-editor/src/components/link-control/make-cancelable.js
Outdated
Show resolved
Hide resolved
packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Kai Hao <[email protected]>
@javierarce and I discussed this and we'll continue all further UI enhancements in follow-up Issues. |
getdave
changed the title
This was referenced Jun 9, 2021
|
For the future, it would be cool if we try to limit the use of snapshots in tests (specially big ones). These tests are harder to debug and change to often when implementation details do. |
@youknowriad I strongly agree with this. Snapshots have their advantages, but often times we were overusing them. A rule of thumb of mine is that if the snapshot doesn't look good when inlining them in the test ( I believe we haven't really reached a consensus about this though, it's still being mentioned/recommended in our doc for testing. It would be good if we can start a new issue, reach a consensus about snapshot testing in general, and update the doc. |
This was referenced Jun 10, 2021
|
Extraordinary work here @getdave ! For me this is a feature which although contained by a small box and only visible on links, it makes the whole experience immensely up to date: rich previews is the expected interaction in a software landscape focused more and more on refined interactions that confirm expected outcomes. 🎉 👏🏻 |
This was referenced Jun 14, 2021
7 tasks
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.
Description
Please note: currently the rich previews are only enabled within the post editor and only then within the format library implementation which is utilised by blocks such as
core/paragraph. This intentional, in order to limit the surface area exposure to the change and allow it to "bed" in. We will enable across other areas of the editor in future subject to it working well where it is currently.Building on #18042 and #31085, this PR wires things together to allow meta data about direct entry URLs (eg:
www.wordpress.org) to be shown in the<LinkControl>UI (the UI that is used to add hyperlinks in Gutenberg).This currently works in the Post Editor richtext only. It will not work (yet!) in
Closes #31466
How has this been tested?
Manual testing and component tests have been completed or added.
To manually test you should:
Screenshots
Screen.Capture.on.2021-06-09.at.16-27-52.mov
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).