Backport PHP parts of sticky position block support #3973

andrewserong · 2023-02-03T01:36:27Z

This PR backports the PHP parts of the new sticky position block support, and is a follow-on from #3865

Backports the PHP parts of the following PRs:

Notes on this backport:

The backport deviates from Gutenberg trunk in one way: fixed position type is excluded from the appearance tools opt-in: this is because fixed positioning isn't being opted-in for the Group block and isn't ready to be used as a default opt-in. The code paths exist for fixed positioning as it will be supported in the future, but for now, sticky is the only one exposed by default. (The overall block support should still support it as a valid value, though)
The styling output while logged in depends on the JS packages update, as it will include the CSS changes in #47665, so while testing this PR you might notice an odd styling issue while logged in. This should be resolved once the package update lands.
With the above caveat shouldn't really be an issue though, as there is no way for a user to set a group block to sticky without the JS packages landing. This PR is to land the required PHP changes in advance of that.

Screenshots:

While logged in (CSS variable that's included in JS packages update isn't yet output in this PR, once it is the sticky block will sit below the admin bar)	While not logged in (correct)

Testing instructions:

In the Group block's block.json file add a "position": { "sticky": true } object under "supports" (Note: for this change to propagate in your testing, you might need to rebuild the block JSON cache locally).
Create a post and add a bunch of paragraphs.
Insert the below markup at the beginning of the post (this is Group block markup with the appropriate position style object to stick to the top of the page when scrolling).
Publish the post, and on the site frontend, the group block should stick to the top of the screen once it has been scrolled past the top of the screen (the block sticks to the scrollable area of its container)

Group block markup for testing purposes:

<!-- wp:group {"style":{"spacing":{"padding":{"top":"1em","right":"1em","bottom":"1em","left":"1em"}},"color":{"background":"#f2aeae"},"position":{"type":"sticky","top":"0px"}},"layout":{"type":"default"}} -->
<div class="wp-block-group has-background" style="background-color:#f2aeae;padding-top:1em;padding-right:1em;padding-bottom:1em;padding-left:1em"><!-- wp:paragraph -->
<p>A paragraph in a sticky group</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

Trac ticket: https://core.trac.wordpress.org/ticket/57618

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

felixarntz

@andrewserong Looks almost good to go, after reviewing this and cross-checking the relevant already approved/merged Gutenberg PRs.

Does this have to wait for the JS package update, or can it be committed to core before already?

src/wp-includes/class-wp-theme-json.php

andrewserong · 2023-02-06T22:29:13Z

Thanks for reviewing @felixarntz!

Does this have to wait for the JS package update, or can it be committed to core before already?

This can be committed before the JS package update, since this PR doesn't opt any of the blocks into the new control, so it doesn't expose anything to a user. I think it's preferable to land this PR first, so that when the JS packages update happens, the PHP parts will already be in place.

mukeshpanchal27

Thanks @andrewserong, Left nit-pick feedback and questions.

mukeshpanchal27 · 2023-02-07T10:33:23Z

src/wp-includes/block-supports/position.php

+ *
+ * @param  string $block_content Rendered block content.
+ * @param  array  $block         Block object.
+ * @return string                Filtered block content.


Suggested change

* @return string Filtered block content.

* @return string Filtered block content.

Per PHP Documentation Standards no needs to add spaces for return doc.

mukeshpanchal27 · 2023-02-07T10:40:55Z

src/wp-includes/block-supports/position.php

+		/*
+		 * Add to the style engine store to enqueue and render position styles.
+		 */


Suggested change

/*

* Add to the style engine store to enqueue and render position styles.

*/

// Add to the style engine store to enqueue and render position styles.

Use a single-line comment instead of a multiline comment for single-line documents.

mukeshpanchal27 · 2023-02-07T10:45:29Z

tests/phpunit/tests/block-supports/wpRenderPositionSupport.php

+		$this->theme_root      = realpath( DIR_TESTDATA . '/themedir1' );
+		$this->orig_theme_dir  = $GLOBALS['wp_theme_directories'];
+
+		// /themes is necessary as theme.php functions assume /themes is the root if there is only one root.


Suggested change

// /themes is necessary as theme.php functions assume /themes is the root if there is only one root.

// '/themes' is necessary as theme.php functions assume '/themes' is the root if there is only one root.

mukeshpanchal27 · 2023-02-07T11:05:03Z

src/wp-includes/block-supports/position.php

+
+	if (
+		! $has_position_support ||
+		empty( $block['attrs']['style']['position'] )


Instead of empty shell we use isset? What do you think?

mukeshpanchal27 · 2023-02-07T11:07:35Z

src/wp-includes/block-supports/position.php

+	if ( true === $theme_has_sticky_support ) {
+		$allowed_position_types[] = 'sticky';
+	}
+	if ( true === $theme_has_fixed_support ) {
+		$allowed_position_types[] = 'fixed';
+	}


Do we needs to bail early if the position is not sticky or fixed?

dream-encode · 2023-02-07T18:03:41Z

Merged into core in https://core.trac.wordpress.org/changeset/55285.

andrewserong added 4 commits February 3, 2023 12:29

Backport PHP parts of sticky position block support

7444fd1

Update appearance tools test

e04c211

Fix linting issue, try to fix style engine output issue

96d8c34

Remove fixed from appearance tools

284edab

andrewserong marked this pull request as ready for review February 3, 2023 05:30

andrewserong mentioned this pull request Feb 3, 2023

Plugin: Backport PHP changes for WordPress 6.2 release WordPress/gutenberg#47187

Closed

85 tasks

ntsekouras requested review from hellofromtonya and dream-encode February 3, 2023 08:07

Update comments

f955fbb

felixarntz reviewed Feb 6, 2023

View reviewed changes

src/wp-includes/class-wp-theme-json.php Show resolved Hide resolved

mukeshpanchal27 reviewed Feb 7, 2023

View reviewed changes

dream-encode closed this Feb 7, 2023

oandregal mentioned this pull request Mar 1, 2023

WP_Theme_JSON classes: clarify status of position.fixed WordPress/gutenberg#48635

Closed

andrewserong mentioned this pull request Mar 1, 2023

Theme JSON: Clarify status of fixed position opt-in in appearance tools WordPress/gutenberg#48660

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport PHP parts of sticky position block support #3973

Backport PHP parts of sticky position block support #3973

andrewserong commented Feb 3, 2023 •

edited

Loading

felixarntz left a comment

andrewserong commented Feb 6, 2023

mukeshpanchal27 left a comment

mukeshpanchal27 Feb 7, 2023

mukeshpanchal27 Feb 7, 2023

mukeshpanchal27 Feb 7, 2023

mukeshpanchal27 Feb 7, 2023

mukeshpanchal27 Feb 7, 2023

dream-encode commented Feb 7, 2023

	* @return string Filtered block content.
	* @return string Filtered block content.

	// /themes is necessary as theme.php functions assume /themes is the root if there is only one root.
	// '/themes' is necessary as theme.php functions assume '/themes' is the root if there is only one root.

Backport PHP parts of sticky position block support #3973

Backport PHP parts of sticky position block support #3973

Conversation

andrewserong commented Feb 3, 2023 • edited Loading

felixarntz left a comment

Choose a reason for hiding this comment

andrewserong commented Feb 6, 2023

mukeshpanchal27 left a comment

Choose a reason for hiding this comment

mukeshpanchal27 Feb 7, 2023

Choose a reason for hiding this comment

mukeshpanchal27 Feb 7, 2023

Choose a reason for hiding this comment

mukeshpanchal27 Feb 7, 2023

Choose a reason for hiding this comment

mukeshpanchal27 Feb 7, 2023

Choose a reason for hiding this comment

mukeshpanchal27 Feb 7, 2023

Choose a reason for hiding this comment

dream-encode commented Feb 7, 2023

andrewserong commented Feb 3, 2023 •

edited

Loading