Block API comment tweaks #5226
Block API comment tweaks #5226
Conversation
mtias
added
[Feature] Block API
| @@ -140,7 +140,7 @@ export function getPossibleBlockTransformations( blocks ) { | |||
| return []; | |||
| } | |||
|
|
|||
| //compute the block that have a from transformation able to transfer blocks passed as argument. | |||
| // Compute the block that have a from transformation able to transfer blocks passed as argument. | |||
There was a problem hiding this comment.
This comment makes little sense :)
blocks/api/serializer.js
Outdated
| // Generated HTML classes for blocks follow the `wp-block-{name}` nomenclature. | ||
| // Blocks provided by WordPress drop the prefixes 'core/' or 'core-' (used in 'core-embed/'). | ||
|
|
||
| // @todo This should be filterable. |
There was a problem hiding this comment.
@gziolo this seems like a good thing to make filterable.
There was a problem hiding this comment.
At the moment you can disable the default class name using supports and className:
// Remove the support for a the generated className .
className: false,See more https://github.com/WordPress/gutenberg/blob/master/docs/block-api.md#supports-optional.
Yes, it seems like we might make it possible to override this function with a custom implementation. I'll open an issue.
| @@ -177,6 +180,7 @@ export function getBeautifulContent( content ) { | |||
| * @return {string} HTML. | |||
| */ | |||
| export function getBlockContent( block ) { | |||
| // @todo why not getBlockInnerHtml? | |||
There was a problem hiding this comment.
@aduth @youknowriad seems we should go with getBlockInnerHtml for consistency.
There was a problem hiding this comment.
Yes, I'd like to align more of the terminology here. This is a good example. Also from the parser output, I'd like to normalize attrs -> attributes, blockName -> name.
gutenberg/blocks/api/parser.js
Lines 186 to 191 in aa1df37
| const blockName = startsWith( rawBlockName, 'core/' ) ? | ||
| rawBlockName.slice( 5 ) : | ||
| rawBlockName; | ||
|
|
||
| // @todo make the `wp:` prefix potentially configurable. |
There was a problem hiding this comment.
Should it be configurable? Or should we just change it per #4636 ? 😄
There was a problem hiding this comment.
I don't think we should change it :)
editor/components/inserter/menu.js
Outdated
| @@ -131,15 +131,15 @@ export class InserterMenu extends Component { | |||
| return recentItems; | |||
|
|
|||
| case 'blocks': | |||
| predicate = ( item ) => item.category !== 'embed' && item.category !== 'reusable-blocks'; | |||
There was a problem hiding this comment.
There's a few more to update:
https://github.com/WordPress/gutenberg/search?utf8=%E2%9C%93&q=reusable-blocks&type=
| innerHTML = innerHTML.trim(); | ||
|
|
||
| // Use type from block content, otherwise find unknown handler. | ||
| name = name || getUnknownTypeHandlerName(); | ||
|
|
||
| // Convert 'core/text' blocks in existing content to the new | ||
| // 'core/paragraph'. | ||
| // Convert 'core/text' blocks in existing content to 'core/paragraph'. | ||
| if ( 'core/text' === name || 'core/cover-text' === name ) { |
There was a problem hiding this comment.
It's probably time to remove this old use-case.
There was a problem hiding this comment.
I was thinking of switching back to text :)
mtias
force-pushed
the
update/block-api-comments
branch
from
0908aa9 to
d9723b9
Compare
| title: __( 'Reusable Block' ), | ||
| category: 'reusable-blocks', | ||
| title: __( 'Shared Block' ), | ||
| category: 'shared', |
There was a problem hiding this comment.
It's a bit odd that we assign a category here, since the block never appears on its own in an inserter. But I guess with our block validation, it's not possible to not assign a category. Maybe we should take isPrivate into consideration when determining whether a category is needed.
Edit: Might also impact blocks which are expected to only be used in a nested context.
Edit 2: We could just make category non-required, though it's easy to mistakenly omit and causes a block to not appear in the inserter.
There was a problem hiding this comment.
I think making category non-required might be the best in any case.
There was a problem hiding this comment.
I would double check if the shared block won’t show up in the inserter with those changes introduced.
I second making category optional and to be expressed as any alphanumerical string. In the inserter we would put it under others category if it isn’t one of those that core provides.
In general I think we should convert shared block logic into the editor UI instead of introducing isPrivate property.
There was a problem hiding this comment.
I think the last time the issue was with the recent blocks in both the regular inserter and in the inline inserter (right hand icons).
Trying to clarify a few intentions with comments more about the why and less about the what. Fixes some spelling issues.