Fetch site icon using the ID from REST API index #43514
Closed
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
Mamaduka
requested review from
priethor and
gziolo
and removed request for
spacedmonkey and
nerrad
Mamaduka
added
[Type] Enhancement
gziolo
added
the
Backport to WP Minor Release
|
Size Change: +93 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
|
This would be an Enhancement for the Gutenberg plugin but a bug fix for WP core. |
|
Everything looks good to me 🙂 ✅ The site icon was displayed in the pre-publish panel. 38dcebf4673ec43d5d7766046f9f434f.mp4 |
|
Thanks for testing, @t-hamano!
It worked like this before; we can look into improving this behavior as a follow-up. |
|
This change will break the solution that was offered to #38555, won't it? By requiring an attachment post ID, plugins or themes will not be able to change the icon to a bundled asset. |
|
@carlomanf, correct. The The plugins/themes can still provide the default favicon using the |
|
Then close this branch and return to the original plan to merge #42957 to core. It will not hurt to have both REST API fields, as they serve different purposes. |
|
This method was recommended by @spacedmonkey, the REST API component maintainer - #42957 (comment). |
|
I'm sure that he would re-consider, if made aware of the issue. |
|
@TimothyBJacobs, I would appreciate your feedback here. Should we try and use the existing |
The choice of words here, "existing," "introduce" and "new," is misleading. The client-side handling for the |
37 tasks
|
No, |
gziolo
removed
the
Backport to WP Minor Release
|
I'm going to close this PR. We decided to go with |
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?
Follow-up #42957.
PR updates method used for fetching site icon to use ID and
getMedia, instead of adding new REST API index.Testing Instructions