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

Add widget-areas endpoint #15015

Merged
merged 1 commit into from
May 16, 2019
Merged

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Apr 17, 2019

Description

This PR uses the work done in #14251 with some adaptations to use CPT being implemented in #15014.

This is a step in a try to implement https://github.com/WordPress/gutenberg/blob/add/blocks-in-widget-areas-rfc/docs/rfcs/blocks-in-widget-areas.md

The CPT itself should be review in #15014.

How has this been tested?

I verified executing this code in the browser console:

wp.apiFetch({
    path: '/__experimental/widget-areas',
    method: 'GET',
}).then(console.log);

Returns the widget areas with widgets being represented as legacy widget blocks.

During the first change, a post is created and the sidebar references that post:

wp.apiFetch({
    path: '/__experimental/widget-areas/sidebar-1',
    data: { content: '<!-- wp:columns --><div class="wp-block-columns has-2-columns"><!-- wp:column --><div class="wp-block-column"><!-- wp:paragraph --><p>a</p><!-- /wp:paragraph --></div><!-- /wp:column --><!-- wp:column --><div class="wp-block-column"><!-- wp:paragraph --><p>brdtertertreterewrew45</p><!-- /wp:paragraph --></div><!-- /wp:column --></div><!-- /wp:columns -->' },
    method: 'POST',
}).then(console.log);

This action is tottaly transparent to the user.

@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 2 times, most recently from b73fb46 to 4722851 Compare April 17, 2019 11:30
@jorgefilipecosta jorgefilipecosta added [Type] Task Issues or PRs that have been broken down into an individual action to take [Feature] Widgets Screen The block-based screen that replaced widgets.php. labels Apr 17, 2019
@jorgefilipecosta jorgefilipecosta changed the base branch from master to add/wordpress-area-cpt April 17, 2019 16:32
@jorgefilipecosta jorgefilipecosta changed the base branch from add/wordpress-area-cpt to master April 17, 2019 16:33
Copy link
Member Author

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

If we update some sidebar to use blocks, the sidebar_options structure is migrated in accordance with the RFC.
This causes some warnings on the existing widgets screen, these warnings may happen in production if the user downgrades to a previous WordPress version. On the same version even on the previous screen, this should not happen as we will implement functionality to handle the new structure even on the previous screen.
image

@jorgefilipecosta
Copy link
Member Author

jorgefilipecosta commented Apr 18, 2019

This PR was updated to be compatible with the current structure used by legacy widget block.
It is possible to test this integration by opening Gutenberg and pasting the following code in the console:

wp.apiFetch({
    path: '/__experimental/widget-areas/sidebar-1',
    method: 'GET',
}).then( ( { content } ) => window.wp.data.dispatch( 'core/editor' ).resetBlocks( window.wp.blocks.parse( content.raw ) ) );

It should load the sidebar widgets in the post editor using the legacy widgets block.

@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 2 times, most recently from 22882cc to 56f0948 Compare April 19, 2019 13:56
@jorgefilipecosta jorgefilipecosta changed the base branch from master to add/wordpress-area-cpt April 19, 2019 16:15
@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-cpt branch 2 times, most recently from 1868ca2 to db638b0 Compare April 30, 2019 14:09
@jorgefilipecosta jorgefilipecosta added the REST API Interaction Related to REST API label May 1, 2019
@jorgefilipecosta jorgefilipecosta marked this pull request as ready for review May 2, 2019 13:24
@jorgefilipecosta jorgefilipecosta changed the base branch from add/wordpress-area-cpt to master May 2, 2019 13:24
@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 3 times, most recently from bffaee5 to 79a8c57 Compare May 2, 2019 18:04
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Thanks for picking this back up! I left some comments. Looking forward to chatting with you about all of this later today. I think the big thing we need to think through is how updating existing legacy widgets should work.

lib/widgets.php Outdated Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Outdated Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Outdated Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Outdated Show resolved Hide resolved
*
* @see WP_REST_Controller
*/
class WP_REST_Widget_Areas_Controller extends WP_REST_Controller {
Copy link
Member

Choose a reason for hiding this comment

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

We ought to add unit tests for all of this.

lib/class-wp-rest-widget-areas-controller.php Outdated Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Show resolved Hide resolved
lib/class-wp-rest-widget-areas-controller.php Outdated Show resolved Hide resolved
@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 8 times, most recently from 877c6b0 to e333625 Compare May 9, 2019 22:03
@jorgefilipecosta
Copy link
Member Author

Hi @noisysocks, thank you for your review 👍 Your feedback was applied!

lib/blocks.php Outdated
* @param array $blocks Post Array of block objects.
* @return string String representing the blocks.
*/
function serialize_blocks( $blocks ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these serialize functions shouldn't be "official" functions we support, As far as I understand, they are only needed for the legacy block behavior. They can be local to that or at least mark them as unstable.

*
* @since 5.7.0
*/
class WP_Widgets_Manager {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be marked experimental as well

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

What's blocking this @noisysocks @jorgefilipecosta I'd love to get a working prototype by the next Gutenberg release.

@jorgefilipecosta
Copy link
Member Author

@youknowriad your comments were addressed. I'm not aware of any blocker and I think we just need the approval of the PR or additional feedback.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Let's move forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. REST API Interaction Related to REST API [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants