-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Make link UI rich previews target agnostic and fix unwanted preview for internal URLs #32658
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
Conversation
Size Change: +266 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
I am aware that all the unit tests are broken. I can fix these as soon as I get a 👍 on the overall direction here. |
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.
Mostly looks good! I'm okay as long as it unblocks things and fixes the bug. We can create follow-up PRs if necessary 👍 .
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
f69622c
to
16b7d61
Compare
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.
LGTM 👍 Just a small question.
Note: |
Description
This PR has two purposes:
#1
, slightly refactor the rich data APIs to make them less external URL specific. This has the advantage of fixing#1
and also preparing the link UI for showing rich previews of internal URLs in the future.Note: changes to test solely down to renaming of the main fetch function.
Closes #32657
Questions
@youknowriad Where do you think the newfetchRichUrlData
function should reside? I was thinking@wordpress/core-data
?Suggest we ship "as is" and revisit where to move the function in a followup.
How has this been tested?
Automated Testing
I will add unit tests for the
fetchRichUrlData
once we have found a suitable home for it.Manual testing instructions
www.wordpress.org
).www.wordpress.org?qs=http://localhost:8888
).tel:123456
).Screenshots
Screen.Capture.on.2021-06-14.at.11-53-07.mp4
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).