#53247 closed task (blessed) (fixed)
Block Editor: Include the Site Logo block from the Gutenberg repository
Reported by: | youknowriad | Owned by: | youknowriad |
---|---|---|---|
Milestone: | 5.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-patch |
Focuses: | Cc: |
Description
Gutenberg 10.7 includes a Site Logo block that requires some updates to the settings endpoint to work properly. For this reason it has been left out of the package update commit.
Change History (16)
This ticket was mentioned in PR #1275 on WordPress/wordpress-develop by aristath.
3 years ago
#1
- Keywords has-patch added
youknowriad commented on PR #1275:
3 years ago
#2
Can we enable the site-logo block in the same patch and add the required code? (I think to do so, there's a couple PHP files/webpack config to change and run npm run build:dev
)
youknowriad commented on PR #1275:
3 years ago
#3
Take the PostAuthor block as an example:
and
youknowriad commented on PR #1275:
3 years ago
#5
@aristath looks like there are some failing phpunit tests here, any idea?
TimothyBJacobs commented on PR #1275:
3 years ago
#7
At a minimum the name for the theme mods option in the REST API should just be theme_mods
. It also should be using set_theme_mod
and probably get_theme_mod
. But again, putting that in this endpoint is entirely incorrect. The capability is incorrect, it is using manage_options
instead of edit_theme_options
and conceptually is a mismatch as the theme mods API is an abstraction over the options API.
Theme mods should either be added to the themes endpoint, or more ideally have an endpoint to interact with registered settings in the customizer.
This is not suitable to be shipped in Core. If you aren't interested in making those endpoints, then I'd recommend looking at calling the customizer admin-ajax API. Ex: https://github.com/WordPress/gutenberg/blob/854a31f3edee98e8b16ef5f2d32abcdaa8b395b0/packages/edit-navigation/src/store/actions.js#L195
This also seems to have a different change of adding the stylesheet
option to the settings endpoint. I'm not sure why this is here, but it can't ship. If you need the active theme get it from the themes endpoint. If you need to change themes, you can't just do that by updating the option. Theme switching needs to be added to the themes endpoint.
3 years ago
#8
For the sake of this PR I updated @wordpress/block-library
to v3.2.2, cleaned-up previous files and updated the ones relevant to the site-logo block.
Tests are currently failing, but it _should_ be OK once the block-library
package gets updated in the master
branch and this gets rebased.
youknowriad commented on PR #1275:
3 years ago
#9
Packages are now up to date on trunk, would you mind refreshing this PR?
youknowriad commented on PR #1275:
3 years ago
#10
The block seems to work in my testing. Any idea about the failing tests? Is it still the action unhooking thing?
3 years ago
#12
All tests now pass. I needed to tweak the sync hook a bit, and a PR backporting these changes to Gutenberg was submitted in https://github.com/WordPress/gutenberg/pull/32370
youknowriad commented on PR #1275:
3 years ago
#13
Ok makes sense. I guess this means we should wait for the next package update to land this PR to avoid having npm run build
override your changes.
#15
@
3 years ago
- Owner set to youknowriad
- Resolution set to fixed
- Status changed from new to closed
In 51091:
youknowriad commented on PR #1275:
3 years ago
#16
committed in https://core.trac.wordpress.org/changeset/51091
Trac ticket: https://core.trac.wordpress.org/ticket/53247