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

Blocks: Support implicit attributes and test them for useColors. #20308

Closed
wants to merge 1 commit into from

Conversation

epiqueras
Copy link
Contributor

@epiqueras epiqueras commented Feb 19, 2020

Description

Gutenberg provides tools for block authors to quickly implement Common Block Functionality.

An example of this is useColors for arbitrary color picking and application.

Most of these tools persist data in the form of new block attributes, which forces developers to add to their block definitions manually.

This PR explores the idea of implicit attributes and leverages them inside useColors to dynamically resolve the new block attribute definitions during block registration.

It does this by monkey patching the React dispatcher to be able to render functional components with hooks to strings (this might also be useful for our block serialization implementation, which currently does not support hooks in block save functions). It then injects an internal hook, useImplicitAttributes, for common block tools or third party custom hooks to register additional attributes during a block's registration. Then it sets everything back to normal before continuing as usual.

It's worth noting that errors are swallowed to avoid potential breakage with blocks that do strange things or depend on context. We can continue to refine the approach as we see run into these edge cases, but blocks can always register attributes explicitly to be safe.

How has this been tested?

It was verified that the paragraph block is still being registered with all color attributes even though they are no longer in its block.json file.

Types of Changes

New Feature: A new API for implicit attributes allows for custom hooks to automatically add attributes to a block's type.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@epiqueras epiqueras added the [Feature] Block API API that allows to express the block paradigm. label Feb 19, 2020
@epiqueras epiqueras self-assigned this Feb 19, 2020
@epiqueras
Copy link
Contributor Author

@mtias, you asked for something like this a while back.

@epiqueras
Copy link
Contributor Author

Another idea is to make these attributes have a distinctive prefix like wp-common-attribute__ and have the parser and serializer always recognize/accept attributes with the prefix (exposing them to the block author without the prefix).

@@ -18,18 +18,6 @@
"placeholder": {
"type": "string"
},
"textColor": {
Copy link
Member

Choose a reason for hiding this comment

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

This is lovely to see :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@@ -245,6 +249,11 @@ export function registerBlockType( name, settings ) {
return;
}

settings.attributes = addImplicitAttributes(
Copy link
Member

Choose a reason for hiding this comment

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

I'd call these supportedAttributes maybe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that.

@mtias mtias mentioned this pull request Feb 20, 2020
82 tasks
@gziolo
Copy link
Member

gziolo commented Feb 21, 2020

I wanted to share a few thoughts for the full picture.

The fun part is that we were blocking using block.json for dynamic blocks, mostly because we have those hooks on the frontend that inject attributes. This proposal goes even further as those attributes would be injected using edit implementation rather than hooks. It also raises the question of what to do when a WP hook overrides the edit implementation after a block was registered, and this logic also uses React hooks?

Another interesting aspect is that there are a few 3rd party plugins that provide some abstraction over attributes to connect them with UI controls, e.g.{ "attributes": { "myAttribute": { "type": "string", "control": "myControlName" } } or something along those lines. So they treat attributes as the source of truth. This proposal goes in the completely opposite direction.

Besides, If we decide that we don’t care about attributes on the server, then what’s the point of keeping attributes in the block.json in the first place?

Another observation is that all the attributes we want to hide from developers aren't strictly related to the content created by the user but rather the impact the visual aspects. align, className – those attributes we handle with WP hooks. They are strictly related to how the block looks on the frontend, and if they got wiped, the content would stay intact. In the context of let's say RSS reader, they don't make much difference. It raises the question, whether font size, text color, or background color shouldn’t be replaced with global styles?

@epiqueras
Copy link
Contributor Author

This implementation could be replicated in the server, but I think I'd prefer a less magical solution like using the prefixes I suggested. What do you think of that?

@ItsJonQ
Copy link

ItsJonQ commented Feb 21, 2020

It raises the question, whether font size, text color, or background color shouldn’t be replaced with global styles?

@gziolo That's a great observation!
I've been looking more into how blocks should "register" (style) attributes they would like globally styled.

Below are some of my thoughts (don't consider them blockers by any means)


Types of Attributes

Using a pre-build mechanism like attributes makes sense. As you've pointed out, there appears to be 2 "types" of attributes (in a way):

Aesthetics vs function.
Design token vs config.

"Font Size" vs something like "Gallery preview type"

Easily registering attributes

The spirit of @epiqueras 's PR (as I see it), hopes to make it easier for blocks (core to custom) to use attributes without having to manually register every single attribute. So things like useColor makes it easier (not sure of implementation/effects, but I agree with the principle ❤️)

Automatically rendered controls?

(Extending things further)

Recently caught up with @mtias! An idea that came about was the (automatic) rendering of controls based on "known"/associated attributes as part of the system.

So by using something like useColors, the controls for colours would automatically appear in the InspectorControl, without edit.js having to manually hook them up to control primitives.


Anywho! Those are my thoughts 😊

Edit: Some of the thoughts above are touched upon here:
#20331

@epiqueras
Copy link
Contributor Author

Aesthetics vs function.
Design token vs config.

Since I can only come up with examples of attributes that we would like to hide of the former kind, and since those examples are now planned to be covered by global style hooks. I'll close this PR.

@epiqueras epiqueras closed this Jun 5, 2020
@youknowriad youknowriad deleted the add/implicit-attributes branch June 5, 2020 08:32
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants