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

Default attributes for block-support properties don't apply on dynamic blocks #50229

Closed
ryelle opened this issue May 1, 2023 · 14 comments
Closed
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@ryelle
Copy link
Contributor

ryelle commented May 1, 2023

I'm building a dynamic block, which supports a handful of the built-in style settings (colors, spacing, etc). I can set defaults in block.json that apply in the editor, but when I view the front end, it's like nothing is set. get_block_wrapper_attributes only returns the block class name, none of the color classes are set, and no style attribute.

It appears that the default attributes don't exist in parsed_block, which is what apply_block_supports uses to render the values in PHP.

Editor, appears correct:

Front end, no styles:

An example block.json

This sets a default width, colors, and padding, but none of those are applied.

{
	"$schema": "https://schemas.wp.org/trunk/block.json",
	"apiVersion": 2,
	"name": "wporg/local-navigation-bar",
	"title": "Local Navigation Bar",
	"icon": "ellipsis",
	"category": "layout",
	"description": "A special block to handle the local navigation on pages in a section.",
	"textdomain": "wporg",
	"attributes": {
		"align": {
			"type": "string",
			"default": "full"
		},
		"backgroundColor": {
			"type": "string",
			"default": "blueberry-1"
		},
		"textColor": {
			"type": "string",
			"default": "white"
		},
		"style": {
			"type": "object",
			"default": {
				"spacing": {
					"padding": {
						"right": "var:preset|spacing|60",
						"left": "var:preset|spacing|60",
						"top": "16px",
						"bottom": "16px"
					}
				}
			}
		}
	},
	"supports": {
		"align": false,
		"color": {
			"text": true,
			"background": true,
			"link": true
		},
		"spacing": {
			"margin": true,
			"padding": true
		},
	},
	"editorScript": "file:./index.js",
	"editorStyle": "file:./editor-style.css",
	"style": "file:./style.css",
	"viewScript": "file:./view.js"
}

I think this is the same root issue as #50027, but it affects more than just the alignment.

@ryelle ryelle added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. labels May 1, 2023
@talldan
Copy link
Contributor

talldan commented May 2, 2023

It appears that the default attributes don't exist in parsed_block, which is what apply_block_supports uses to render the values in PHP.

From what I understand, the default are only usually returned when using an instance of the WP_Block:
https://github.com/WordPress/wordpress-develop/blob/1bd815adbbbb48d486a5519ed2a3d34eefe6a407/src/wp-includes/class-wp-block.php#L165-L192

This looks like the code that sets the $parsed_block:
https://github.com/WordPress/wordpress-develop/blob/1bd815adbbbb48d486a5519ed2a3d34eefe6a407/src/wp-includes/class-wp-block.php#L252-L263

I don't really know the code well enough to offer suggestions on how to fix it in a backward compatible way. This code has been untouched for years by the looks of it, but @youknowriad last modified it, so might be able to offer suggestions.

@youknowriad
Copy link
Contributor

How do you set defaults in block.json? Do you redeclare the style attribute manually?


This issue looks like a symptom of the fact that the block parsing in the server is not done "fully". It's just the grammar parse. Unfortunately, there's no solution for that at the moment as introducing the full HTML parsing is too impactful.

I'm not really sure why we made sure the default attributes available when using $WP_Block instance, this feels like a hacky fix for me but there's a precedent, maybe we can find a way to use that in apply_block_supports as well?

@ryelle
Copy link
Contributor Author

ryelle commented May 2, 2023

How do you set defaults in block.json? Do you redeclare the style attribute manually?

My block.json is in the issue description. I'm setting some attributes directly, and for spacing, I set style, following the docs for using block supports.

If it helps, for output my render function is simply:

function render( $attributes, $content, $block ) {
	$wrapper_attributes = get_block_wrapper_attributes();
	return sprintf(
		'<div %1$s>%2$s</div>',
		$wrapper_attributes,
		$content
	);
}

@dutchigor
Copy link

I'm encountering the same issue. See a minimal plugin demonstrating the issue here: https://github.com/dutchigor/wordpress-block-supports-render-callback-test

@colorful-tones
Copy link
Member

I just hit this issue today. I would be keen to see some traction.

@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Nov 14, 2023
@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Nov 20, 2023

This issue was also reported in this ticket: here. I've been able to reproduce it as well. Any clue/idea how can this be fixed?

@youknowriad
Copy link
Contributor

The solution fix can be as I explained here #50229 (comment)

Make sure to include the default attributes to the "attributes" that are passed to apply_block_supports calls. I don't like this fix personally but also don't like the same fix that is present in WP_Block instance either (not sure when this was introduced) because it feels like we're hacking the code because we don't have a full "parser" in the server.

One potential approach as well would be to update parse_blocks in the server to always include the default attributes but it can be seen as a small breaking change as well.

@spencerfinnell
Copy link

spencerfinnell commented Feb 19, 2024

I am running in to this as well.

If a block's style attribute has been touched/modified the attribute is saved in the block markup, otherwise it's not. Then accessing $attributes in the render_callback shows a mix of the default block.json value (unparsed) and the CSS preset variable that corresponds with the editor setting:

// Attribute is unchanged in the editor.
["style"]=>
   array(1) {
    ["spacing"]=>
      array(1) {
        ["blockGap"]=>
          string(7) "0.44rem"
        }
    }
// Attribute is changed in the editor.
["style"]=>
   array(1) {
    ["spacing"]=>
      array(1) {
        ["blockGap"]=>
          string(21) "var:preset|spacing|20"
        }
    }

Some additional discussion about default style attributes in block.json: #53603

@onetrev
Copy link

onetrev commented Sep 3, 2024

To reduce the number of open issues, just wondering if this should actually be marked as a duplicate of this very old issue?

Also, if this is not going to be fixed soon can we please document this bug, maybe here? This would help prevent hours lost desperately trying to figure out why you can't get something seemingly so simple to work. Not that I did that myself. 🤪

@gziolo
Copy link
Member

gziolo commented Sep 25, 2024

To reduce the number of open issues, just wondering if this should actually be marked as a duplicate of this #7342?

I don't think it's the same issue. In fact, I must say that #7342 is an expected behavior because the default value by design is never serialized in the block metadata.

Make sure to include the default attributes to the "attributes" that are passed to apply_block_supports calls. I don't like this fix personally but also don't like the same fix that is present in WP_Block instance either (not sure when this was introduced) because it feels like we're hacking the code because we don't have a full "parser" in the server.

That's the fix that I'm working on right now, and it resolves the issue:

diff --git a/src/wp-includes/class-wp-block-supports.php b/src/wp-includes/class-wp-block-supports.php
index c90b5e0c54..71d6b49691 100644
--- a/src/wp-includes/class-wp-block-supports.php
+++ b/src/wp-includes/class-wp-block-supports.php
@@ -104,7 +104,7 @@ class WP_Block_Supports {
 		}
 
 		$block_attributes = array_key_exists( 'attrs', self::$block_to_render ) && is_array( self::$block_to_render['attrs'] )
-			? self::$block_to_render['attrs']
+			? $block_type->prepare_attributes_for_render( self::$block_to_render['attrs'] )
 			: array();
 
 		$output = array();

I agree with the sentiments shared by @youknowriad that it is suboptimal, but at the same time simple and effective. In the long run, we should rewrite the block parser. @dmsnell did some initial exploration for the lazy parser, that could absorb such operations like adding the default values automatically whenever the attribute is requested for the first time:

@gziolo gziolo added [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. and removed Needs Dev Ready for, and needs developer efforts labels Sep 25, 2024
@gziolo
Copy link
Member

gziolo commented Sep 25, 2024

Fix: WordPress/wordpress-develop#7438
Trac Ticket: https://core.trac.wordpress.org/ticket/62114

@gziolo gziolo self-assigned this Sep 25, 2024
@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Sep 25, 2024
@gziolo
Copy link
Member

gziolo commented Sep 26, 2024

It should be resolved after WordPress 6.7 gets released in November. Related commit: WordPress/wordpress-develop@7d0e751.

@gziolo gziolo closed this as completed Sep 26, 2024
@youknowriad
Copy link
Contributor

Thanks @gziolo I appreciate the fix.

@onetrev
Copy link

onetrev commented Sep 27, 2024

Amazing, thank you @gziolo. This will be very helpful (once WP 6.7 lands). 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
Development

No branches or pull requests

9 participants