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: Match blocks in the inserter using keywords from patterns #19243

Open
wants to merge 10 commits into
base: master
from

Conversation

@gziolo
Copy link
Member

gziolo commented Dec 19, 2019

Description

Part of #16283.

This is a prototype that illustrates how we can match blocks using registered patterns. It still needs more work on the UI side of things but first, we need to clarify what options we want to give for plugin developers or site owners.

Problem 1

Users enter specific search criteria, and after selecting their block, still need to go through a step to define the layout structure which was already defined in their search.

Ex. User searches for "3 column equal split" and sees the Columns block in the results. When adding the Columns block they still get the placeholder "setup" options to select which type of column layout they want.

Solution

When user defines the layout pattern that a block provides in their search, we can skip the "setup" state altogether. So, the user searches for a specific pattern option that the block provides. User adds the block to their page, and because their search matches one of the patterns, we skip the layout option step and just add the block with their requested pattern automatically.

Problem 2

A site or plugin owner doesn't like the default state of the block. They would like to customize some of the attributes of inner blocks before the block is even inserted.

Solution

Custom code changes the defaults for the blocks and it even allows to present multiple variations of the same block in the inserter ready to be used right away.

Testing scenarios:

  1. Registers the pattern with the inner blocks to apply from the inserter using scope limited to the inserter:
wp.blocks.__experimentalRegisterBlockPattern( 'core/columns', { name: 'custom', title: 'Smiley', isDefault: true, innerBlocks: [ [ 'core/column' ], [ 'core/column' ], [ 'core/column' ], [ 'core/column' ] ], icon: 'smiley', scope: [ 'inserter' ] } );

patterns-api-inserter-1

patterns-api-inserter-2

  1. Registers two patterns with initial attributes to apply from the inserter directly:
wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'green-text', title: 'Green Text', description: 'This block has green text. It overrides the default description.'  attributes: { content: 'Green Text', textColor: 'vivid-green-cyan' }, icon: 'palmtree', scope: [ 'inserter' ] } );
wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'red-text', title: 'Red Text', attributes: { content: 'Red Text', level: 3, textColor: 'vivid-red' }, icon: 'smiley', scope: [ 'inserter' ] } );

patterns-api-inserter-3

  1. Registers a pattern which allows changing the default style variation:
wp.blocks.__experimentalRegisterBlockPattern( 'core/quote', { name: 'large', title: 'Large', isDefault: true, attributes: { className: 'is-style-large' }, icon: 'palmtree', scope: [ 'inserter' ] } );

Changes Covered

This PR integrates the experimental Patterns API with the inserter. This means that defined patterns are queryable in the inserter and you can match blocks using the existing fields plus all pattern names. For simplicity, I implemented the prototype to multiple the block type item by the number of the matched patterns (in case that a block is matched but not its patterns, it displays all patterns).

In addition, this proposal introduces scopes for the block patterns:

  • inserter - show pattern only in the inserter
  • block - show pattern only in the pattern picker rendered when the block is inserted
    When no scope is defined, the patter is available in all places.

Open Questions

  1. What do we do about the slash inserter? Should it also list the patterns registered for blocks? How would that work?
  2. What about the default block if it gets patterns defined with the inserter scope?
  3. Do we really need the inserter scope? Maybe it would be fine to have all or the block? Unless we still provide a way to list the block in the inserter without any pattern to let users see the patterns picker.
  4. Should we update the block preview with the pattern applied?
@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Dec 19, 2019

I anticipate that the search logic needs to be updated to provide more details on how a given block was matched.

Do we need design focus on this?

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 13, 2020

I anticipate that the search logic needs to be updated to provide more details on how a given block was matched.

Do we need design focus on this?

Yes, it would be cool to get designers involved. @mapk can you help with that? In #16283 @kjellr proposed a few approaches in #16283 (comment). I want to try first with:

When there's a matching layout/pattern, a simple option would be to simply write out the template name underneath the block name. Then, if a user selects that item, we could serve up the matching template by default, and they could skip the layout-picking step:

Opt1-A

@mapk

This comment has been minimized.

Copy link
Contributor

mapk commented Jan 13, 2020

I'm going to try and break this down a bit, but please correct me if I'm wrong.

Problem

Users enter specific search criteria, and after selecting their block, still need to go through a step to define the layout structure which was already defined in their search.

Ex. User searches for "3 column equal split" and sees the Columns block in the results. When adding the Columns block they still get the placeholder "setup" options to select which type of column layout they want.

Solution

When user defines the layout pattern that a block provides in their search, we can skip the "setup" state altogether. So, the user searches for a specific pattern option that the block provides. User adds the block to their page, and because their search matches one of the patterns, we skip the layout option step and just add the block with their requested pattern automatically.

Questions

  • Does this flow work with anything other than the Columns block right now?
  • When I search for "3 column table," do I see an option for both the Columns block and the Table block? If, "yes," and I select the Table block, does it automatically create a 3 column table for me... with how many rows?
  • If we show icons of each of the layouts that a particular block offers, is it helpful to show that icon in the search results instead of the generic block's icon? So when searching for " 2 column equal split" I see the 2 column equal split icon instead of the generic 3 column icon for the Columns block. <-- This sounds to me like where the design request comes in to play.
@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from b05a375 to b93c846 Jan 14, 2020
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 14, 2020

I updated the branch with the very naive UI changes to highlight matched patterns:

Screen Shot 2020-01-14 at 19 18 53

We will have to further work on the matching algorithm for the patterns, at the moment it gets highlighted when at least one term is matched.

@mapk I will respond to your questions tomorrow. In short, I believe that we need much complex approach:

  • it should be possible to define whether the given pattern should show up in the inserter or in the block canvas, or in both places
  • it would be neat to be able to use patterns to override attributes for the inserted block, so block itself would show up in the inserter as is, but site owners or plugin developers could tweak how it gets inserted, e.g, the Paragraph block could have background and text color pre-selected
@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from b93c846 to 76710c6 Jan 14, 2020
@youknowriad youknowriad mentioned this pull request Jan 15, 2020
3 of 9 tasks complete
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 15, 2020

For the sake of testing, patterns can be registered for the Columns block to match in the inserter:

wp.blocks.__experimentalRegisterBlockPattern( 'core/columns', { name: 'custom', label: 'Smiley', isDefault: true, innerBlocks: [ [ 'core/column' ], [ 'core/column' ], [ 'core/column' ], [ 'core/column' ] ], icon: 'smiley' } );

Then it will show up when you type smiley.

I added the scope to patterns and marked all currently existing patterns for the Columns block as block only, which means they can't be matched in the inserter.

I should wrap up the exploration tomorrow and I will share my thoughts. The largely align with what @mtias summarized in this comment: #16283 (comment)

@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from 76710c6 to a9e5124 Jan 15, 2020
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 16, 2020

Some testing scenarios to look at with the latest changes applied to this branch. I think this is the baseline that opens all the existing blocks for very nice customization.

  1. Registers the pattern with the inner blocks to apply from the inserter using scope limited to the inserter:
wp.blocks.__experimentalRegisterBlockPattern( 'core/columns', { name: 'custom', label: 'Smiley', isDefault: true, innerBlocks: [ [ 'core/column' ], [ 'core/column' ], [ 'core/column' ], [ 'core/column' ] ], icon: 'smiley', scope: [ 'inserter' ] } );

patterns-api-inserter-1

patterns-api-inserter-2

  1. Registers two patterns with initial attributes to apply from the inserter directly:
wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'green-text', label: 'Green Text', isDefault: true, attributes: { content: 'Green Text', textColor: 'vivid-green-cyan' }, icon: 'palmtree', scope: [ 'inserter' ] } );
wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'red-text', label: 'Red Text', attributes: { content: 'Red Text', level: 3, textColor: 'vivid-red' }, icon: 'smiley', scope: [ 'inserter' ] } );

patterns-api-inserter-3

@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from a9e5124 to 47f4a06 Jan 16, 2020
@gziolo gziolo requested review from mtias and mapk Jan 16, 2020
@gziolo gziolo marked this pull request as ready for review Jan 16, 2020
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 16, 2020

  • Does this flow work with anything other than the Columns block right now?

In the core, only Columns block is affected, so all these changes are safe to land since its experimental API.

  • When I search for "3 column table," do I see an option for both the Columns block and the Table block? If, "yes," and I select the Table block, does it automatically create a 3 column table for me... with how many rows?

With the scope added, you no longer can search in the inserter for the core supported patterns for the Columns block. Now, if you would register them for the inserter for both blocks with some initial data to fill, then it might work or be broken, it depends on the block implementation :)

  • If we show icons of each of the layouts that a particular block offers, is it helpful to show that icon in the search results instead of the generic block's icon? So when searching for " 2 column equal split" I see the 2 column equal split icon instead of the generic 3 column icon for the Columns block. <-- This sounds to me like where the design request comes in to play.

I applied this suggestion since I decided to clone the block type for each pattern. It's still something we should agree on. I think the current shape, the code is very flexible to apply all type of changes to support design needs. We can also do it separately given that you need to explicitly register patterns to have any impact on the inserter.

@apeatling

This comment has been minimized.

Copy link
Contributor

apeatling commented Jan 16, 2020

Nice work!

Should we update the block preview with the pattern applied?

My thought having seen this is it would be ideal to update the preview in the inserter with the pattern applied. You might also want to update the description? In the contact form example above, the description could be very specific to that use case.

I'm not coming in with a ton of context, but is it suggested that if I select a pattern from the picker I would not be able to switch patterns in the sidebar after inserting?

If that's correct, in this scenario why not allow complete customization of the name? From the perspective of the user I'm not sure they would care that it's a "form block with contact template applied", or a "columns block with two columns equal split template applied". What if instead you could just set the name as "Contact Form", or "Two Equal Columns" for these?

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 17, 2020

This already feels like it can accommodate a lot of purposes, nice work.

One thing that stands out for me is that the canonical block — i.e., the block according to its type with no variation applied — disappears from the Inserter the moment at least one pattern exists for that block type. This feels particularly unexpected with the Heading block example, in that after calling registerBlockPattern we "lose" such a basic block.

However, for certain cases I understand the interest in letting variations supersede the canonical block. But that may be an explicit choice. I am unsure where the burden of this choice should lie, but it is probably tied to the semantics of variations' isDefault property.

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 17, 2020

One thing that stands out for me is that the canonical block — i.e., the block according to its type with no variation applied — disappears from the Inserter the moment at least one pattern exists for that block type. This feels particularly unexpected with the Heading block example, in that after calling registerBlockPattern we "lose" such a basic block.

"What about the default block if it gets patterns defined with the inserter scope?" - I mentioned this open question in the description. It's a design decision and I'm happy to update the code to whatever is decided.

But that may be an explicit choice. I am unsure where the burden of this choice should lie, but it is probably tied to the semantics of variations' isDefault property.

We could also do the following, leave the bare block type listed when no matched pattern has isDefault set to true. Otherwise, it means that the custom code opted out from the bare block type in favor of the default variation.

My thought having seen this is it would be ideal to update the preview in the inserter with the pattern applied. You might also want to update the description? In the contact form example above, the description could be very specific to that use case.
[...]
If that's correct, in this scenario why not allow complete customization of the name?

I'm about to land a commit where I manage to apply attributes from the variation to the preview. It's tricky for innerBlocks as they are different conceptually to what you get from the example block property.

Screen Shot 2020-01-17 at 13 46 53

It's something we could do for the block type item and its preview in the inserter to completely replace:

  • name
  • icon
  • description (to be added in that case to the implementation)

We could even allow overriding the provided example.

I'm not coming in with a ton of context, but is it suggested that if I select a pattern from the picker I would not be able to switch patterns in the sidebar after inserting?

It's still to be explored, definitely something on the table and listed in the parent issue as one of the features.

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 17, 2020

"What about the default block if it gets patterns defined with the inserter scope?" - I mentioned this open question in the description. It's a design decision and I'm happy to update the code to whatever is decided.

👍👍

We could also do the following, leave the bare block type listed when no matched pattern has isDefault set to true. Otherwise, it means that the custom code opted out from the bare block type in favor of the default variation.

Great! Let's try it and see how it feels.

@mtias

This comment has been minimized.

Copy link
Contributor

mtias commented Jan 17, 2020

It's a design decision and I'm happy to update the code to whatever is decided.

I think this would always have exceptions so should be configurable. For example, a vanilla social-icon-item block wouldn't be of use in the inserter compared to the actual service icon, but other blocks should always exist as their default state. Embeds is a good middle ground where it wants both the services exposed and the generic embed blocks exposed.

I'd probably look at making it opt-out (maybe tied to inserter: false somehow on the parent?).

@mtias

This comment has been minimized.

Copy link
Contributor

mtias commented Jan 17, 2020

(Side note, we should do a comprehensive review of the API once all the pieces are in place to see if any abstractions have gone too far in the flexibility / clarity scale.)

@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from b8b69a0 to 868ac92 Jan 23, 2020
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 23, 2020

Thank you all for your great feedback. In particular, many thanks @mtias and @mcsf for your help with the processing all the use cases in the chats in different places.

This is what I'm about to push to this branch once I resolve all failing tests and all that jazz:

wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'green-text', label: 'Green Text', isDefault: false, description: 'I like my description more :)', attributes: { content: 'Green Text', textColor: 'vivid-green-cyan' }, icon: 'palmtree', scope: [ 'inserter' ] } );
wp.blocks.__experimentalRegisterBlockPattern( 'core/heading', { name: 'red-text', label: 'Red Text', attributes: { content: 'Red Text', level: 3, textColor: 'vivid-red' }, icon: 'smiley', scope: [ 'inserter' ] } );

Screen Shot 2020-01-23 at 18 00 42

Switching isDefault to true in the test snippet changes all that to:

Screen Shot 2020-01-23 at 18 04 37

Green has description override defined, red defines only label and icon.

I also added an option to override the example, it might be useful for inner blocks overriding. In the case of attributes, they are applied from the pattern directly to the existing example. I'm fine with making it less opinionated.

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 23, 2020

Tomorrow I would like to address:

Otherwise, I think it's time to land this.

In a follow-up PRs, I would like to:

  • rename patterns to variations as proposed by @mtias
  • improve the matching algorithm for search terms taking into account the fact that variations override titles, we also discuss allowing keywords for variations, e.g.:
    Screen Shot 2020-01-24 at 11 28 26
@gziolo gziolo force-pushed the update/blocl-patterns-inserter-integration branch from 721026e to d56e687 Jan 24, 2020
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Jan 24, 2020

@mapk - I would appreciate your feedback on whether it's fine to land it as is and iterate on the design separately. It doesn't affect how Gutenberg works at all until 3rd party code registers those patterns (soon variations).

I updated examples in the PR description to reflect all changes applied.

@mcsf

This comment has been minimized.

Copy link
Contributor

mcsf commented Jan 24, 2020

I pushed d11009e so that the auto-insert block introduced in #16708 takes block variations into account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.