Skip to content
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

REST API: Add the missing 'site_icon_url' to the index #42957

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Aug 4, 2022

What?

Fixes #42943.
Regressed in #41306.

Fixes bug when site logo doesn't replace WordPress logo in the toolbar. We forgot to backport the site_icon_url REST index into the core.

Why?

It was feature regression.

How?

Adds the site_icon_url index using rest_index. Alternatively, we can update components to use the site_icon ID and the fetch image on the client side.

Testing Instructions

  1. Go to Appearance > Editor.
  2. Add a Site Logo block.
  3. Add a logo.
  4. In the block settings for the Site Logo block, toggle on the "Use as site icon" option.
  5. Refresh and confirm that the W icon updates in the Navigation button.

Screenshots or screencast

CleanShot 2022-08-04 at 09 30 28

@Mamaduka Mamaduka requested a review from spacedmonkey as a code owner August 4, 2022 05:38
@Mamaduka Mamaduka requested review from ntsekouras and gziolo and removed request for spacedmonkey August 4, 2022 05:38
@Mamaduka Mamaduka self-assigned this Aug 4, 2022
@Mamaduka Mamaduka added General Interface Parts of the UI which don't fall neatly under other labels. [Type] Regression Related to a regression in the latest release [Block] Site Logo Affects the Site Logo Block labels Aug 4, 2022
Copy link
Contributor

@priethor priethor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected; thanks for the quick turnaround!

image

@Mamaduka Mamaduka merged commit 47b1d50 into trunk Aug 4, 2022
@Mamaduka Mamaduka deleted the fix/rest-api-site-logo-url branch August 4, 2022 09:25
@Mamaduka Mamaduka added the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Aug 4, 2022
@github-actions github-actions bot added this to the Gutenberg 13.9 milestone Aug 4, 2022
@priethor priethor removed the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Aug 4, 2022
priethor pushed a commit that referenced this pull request Aug 5, 2022
* REST API: Add the missing 'site_icon_url' to the index
* Add backport note
@priethor
Copy link
Contributor

priethor commented Aug 5, 2022

Cherry-picked and released in Gutenberg 13.8.1. ✅

@gziolo gziolo added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 11, 2022
@gziolo
Copy link
Member

gziolo commented Aug 22, 2022

@Mamaduka, is it something that has been backported to WordPress core already?

@Mamaduka
Copy link
Member Author

@gziolo, no, this hasn't been backported yet.

@spacedmonkey
Copy link
Member

This change is not needed. You already have site icon in the index. See WordPress/wordpress-develop@aed3eae

Just embed the site icon. This has been there since 5.8

@Mamaduka
Copy link
Member Author

Thanks for the feedback, @spacedmonkey.

The site_icon_url index was supposed to ship with WP 5.6 but was never backported into the core- #22952.

I'm fine using the existing index and not introducing a new one. @gziolo, what do you think?

@gziolo
Copy link
Member

gziolo commented Aug 23, 2022

I'm fine using the existing index and not introducing a new one. @gziolo, what do you think?

Yes, that would be better to reuse the existing setting.

@Mamaduka
Copy link
Member Author

Here's a follow-up PR to use the recommended method - #43514.

@gziolo gziolo removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 23, 2022
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 6, 2022
The `site_icon_url` index was supposed to ship with WordPress 5.6, but was [WordPress/gutenberg#22952 never backported to core].

This commit backports the original PR from Gutenberg repository:
* [WordPress/gutenberg#42957 #42957: REST API: Add the missing 'site_icon_url' to the index]

Follow-up to [52080].

Props Mamaduka, bernhard-reiter, TimothyBlynJacobs.
See #56467.
Built from https://develop.svn.wordpress.org/trunk@54083


git-svn-id: http://core.svn.wordpress.org/trunk@53642 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Sep 6, 2022
The `site_icon_url` index was supposed to ship with WordPress 5.6, but was [WordPress/gutenberg#22952 never backported to core].

This commit backports the original PR from Gutenberg repository:
* [WordPress/gutenberg#42957 #42957: REST API: Add the missing 'site_icon_url' to the index]

Follow-up to [52080].

Props Mamaduka, bernhard-reiter, TimothyBlynJacobs.
See #56467.
Built from https://develop.svn.wordpress.org/trunk@54083


git-svn-id: https://core.svn.wordpress.org/trunk@53642 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pbearne pushed a commit to pbearne/wordpress-develop that referenced this pull request Sep 9, 2022
The `site_icon_url` index was supposed to ship with WordPress 5.6, but was [WordPress/gutenberg#22952 never backported to core].

This commit backports the original PR from Gutenberg repository:
* [WordPress/gutenberg#42957 #42957: REST API: Add the missing 'site_icon_url' to the index]

Follow-up to [52080].

Props Mamaduka, bernhard-reiter, TimothyBlynJacobs.
See #56467.

git-svn-id: https://develop.svn.wordpress.org/trunk@54083 602fd350-edb4-49c9-b593-d223f7449a82
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this pull request Sep 18, 2022
The `site_icon_url` index was supposed to ship with WordPress 5.6, but was [WordPress/gutenberg#22952 never backported to core].

This commit backports the original PR from Gutenberg repository:
* [WordPress/gutenberg#42957 #42957: REST API: Add the missing 'site_icon_url' to the index]

Follow-up to [52080].

Props Mamaduka, bernhard-reiter, TimothyBlynJacobs.
See #56467.
Built from https://develop.svn.wordpress.org/trunk@54083
ootwch pushed a commit to ootwch/wordpress-develop that referenced this pull request Nov 4, 2022
The `site_icon_url` index was supposed to ship with WordPress 5.6, but was [WordPress/gutenberg#22952 never backported to core].

This commit backports the original PR from Gutenberg repository:
* [WordPress/gutenberg#42957 #42957: REST API: Add the missing 'site_icon_url' to the index]

Follow-up to [52080].

Props Mamaduka, bernhard-reiter, TimothyBlynJacobs.
See #56467.

git-svn-id: https://develop.svn.wordpress.org/trunk@54083 602fd350-edb4-49c9-b593-d223f7449a82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Site Logo Affects the Site Logo Block General Interface Parts of the UI which don't fall neatly under other labels. [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site Logo Block: when set as Site Icon, W menu doesn't update
4 participants