#48529 closed enhancement (fixed)
Add fields to WP_Block_Type
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 5.0 |
Component: | Editor | Keywords: | has-patch has-dev-note has-unit-tests commit |
Focuses: | Cc: |
Attachments (5)
Change History (31)
#2
@
5 years ago
This looks great, what's necessary to test it and land it? Does it require changes in unit tests?
#3
@
5 years ago
Good question @gziolo . I will take a look. But the tests should be simple.
We also need to add the fields in for currently registered blocks. I made a start with 48529-implement.diff.
We will also need to update the social links in the plugin.
#4
follow-up:
↓ 5
@
5 years ago
There's at least one condition which tests isset
on a property of this class, where we're now always assigning a default. Do those need to be updated?
#5
in reply to:
↑ 4
@
5 years ago
Replying to aduth:
There's at least one condition which tests
isset
on a property of this class, where we're now always assigning a default. Do those need to be updated?
What if someone doesn't register attributes in their plugin for their custom block. That check might be overkill, but might stop a breakable if someone does something silly.
#6
@
5 years ago
But unless I'm mistaken, since the patch proposes to assign a default value for $attributes
, that condition could never be true
(unless someone went out of their way to unset( $block_type->attributes )
).
#9
follow-up:
↓ 10
@
5 years ago
https://core.trac.wordpress.org/attachment/ticket/48529/48529.diff looks good, one note:
'title' => $this->name,
Name and title are 2 different things.
https://core.trac.wordpress.org/attachment/ticket/48529/48529-implement.diff - this should be done on the Gutenberg side because those files are copied over from node_modules
folder.
#10
in reply to:
↑ 9
@
5 years ago
- Keywords has-dev-note needs-unit-tests added
- Milestone changed from Awaiting Review to 5.5
https://core.trac.wordpress.org/attachment/ticket/48529/48529-implement.diff - this should be done on the Gutenberg side because those files are copied over from
node_modules
folder.
Never really thought this would go in the commit, which is it why it is a different patch file.
This needs to tests.
#11
@
5 years ago
Two more things:
Parent set as an empty array prevents the block from appearing in the inserter, it should remain undefined:
https://developer.wordpress.org/block-editor/developers/block-api/block-registration/#parent-optional
Example is an object in JavaScript, so here it probably should be an array rather than boolean, it should be undefined by default:
https://developer.wordpress.org/block-editor/developers/block-api/block-registration/#example-optional
#12
@
5 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
Updated the patch for 5.5. Can you review again @gziolo ?
#13
@
5 years ago
- Keywords commit added
Nice, it looks great, let's get it in.
One non-blocking nitpick regarding the test:
'text_domain' => [ 'test_domain' ],
It should be a string rather than an array.
#14
in reply to:
↑ 7
@
5 years ago
Replying to spacedmonkey:
Good point @aduth.
I can remove that check.
Was this ever addressed? I don't see a change in the latest patch.
#15
@
5 years ago
One more thing, it looks like textdomain
is the preferred way of spelling it:
https://github.com/WordPress/wordpress-develop/search?q=textdomain&unscoped_q=textdomain
Should we follow it?
#16
@
5 years ago
Oh, and we should use styles
without the alias for styleVariations
as discussed in https://github.com/WordPress/gutenberg/pull/22519#discussion_r430981909
#17
@
5 years ago
I applied a few changes with the latest patch version uploaded:
- renamed
text_domain
totextdomain
(seems to be preferred in WordPress core, https://github.com/WordPress/wordpress-develop/search?q=textdomain&unscoped_q=textdomain) and changed the default value tonull
to reflect the fact that it can be ommited - removed all references to
styleVariations
to continue usestyles
that existed before - updated the default value for
attributes
tonull
to make it work with the logic pointed by @aduth – we can change it later but I would prefer to keep it out of this ticket because it's very sensitive change - updated
parent
to allownull
value and make it the default - it makes it possible to distinguish between[]
andnull
that can have different meanings as pointed in https://github.com/WordPress/gutenberg/pull/22695#issuecomment-635329233 by @aduth example
is also sensible tonull
vs[]
logic
#20
@
5 years ago
There is some conflict with how the block registration considers these new default values. Debugging details are summarized at the GitHub issue:
#22
@
5 years ago
@aduth, I included a new diff that in combination with https://github.com/WordPress/gutenberg/pull/22849 should resolve the issues you raised.
#25
@
5 years ago
In [48117]:
Blocks: Add context fields to WP_Block_Type
New block context related fields were added as part of https://github.com/WordPress/gutenberg/pull/22686. This changest backports them to WP_Block_Type
class.
This ticket is a blocker for #47620.