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

Block library: Patterns API integration with Media & Text block #18343

Closed

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Nov 6, 2019

Description

Builds on top of #18283.

Part 3.a of #16283.

This PR explores a possible way to integrate Patterns API with the existing Media & Text block as proposed in in #16283 (comment)

Example - Media Text

As part of the implementation process, I reorganized the folder structure of the Media & Text block to group together all files related to the edit component.

How has this been tested?

See the recorder screencast.

  1. Open a new post.
  2. Insert Media & Text block.
  3. Pick one of the patterns and make sure it gets applied.

Screenshots

Screen Shot 2019-11-06 at 19 37 01

media-text

Types of changes

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.

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Status] In Progress Tracking issues with work in progress [Package] Block library /packages/block-library labels Nov 6, 2019
@gziolo gziolo self-assigned this Nov 6, 2019
@gziolo gziolo added the [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible label Nov 6, 2019
@gziolo
Copy link
Member Author

gziolo commented Nov 7, 2019

What I struggle with at the moment is that there is no formal way to detect whether the inserted block is in the mode where the user should pick the pattern. When you insert Media & Text it presents you some patterns to pick from, but when you pick one of them, save and reload, the pattern picker will show up again.

@gziolo gziolo force-pushed the update/columns-block-patterns-api branch from c659e2e to 5744f19 Compare November 8, 2019 07:02
@gziolo gziolo force-pushed the update/media-text-patterns-api branch from 00d2e47 to 81e6313 Compare November 8, 2019 07:15
@gziolo
Copy link
Member Author

gziolo commented Nov 8, 2019

What I struggle with at the moment is that there is no formal way to detect whether the inserted block is in the mode where the user should pick the pattern. When you insert Media & Text it presents you some patterns to pick from, but when you pick one of them, save and reload, the pattern picker will show up again.

I was able to reproduce the behavior of the Columns block as suggested by @aduth on Slack, the related code is here:
https://github.com/WordPress/gutenberg/blob/59da6ed/packages/block-library/src/columns/edit.js#L211-L265

@mapk, @karmatosed or @jasmussen - I recorded a screencast to present how it might work based on the proposal shared in #16283 (comment)

Example - Media Text

Should we proceed with that idea? We would need to provide final icons.

media-text

@gziolo gziolo added Needs Design Feedback Needs general design feedback. and removed [Status] In Progress Tracking issues with work in progress labels Nov 8, 2019
@jasmussen
Copy link
Contributor

I'm exceptionally late to this one, but I'm not sure it's any kind of a good pattern to use placeholders for everything as more or less outlined in #16283 (comment). It is appropriate for many things, like the columns block, perhaps even the table block, because it's somewhat difficult to configure those. But to me it fundamentally feels wrong to make the Media & Text block have a setup state at all. This gets even weirder because the block itself includes an Image block that is also in a setup state. So suddenly it simply adds wizardy steps to add a Media & Text block. Looking at https://user-images.githubusercontent.com/1202812/60599876-30666300-9d7d-11e9-9cf2-ec5cc3d02e9b.png it's even unclear what the difference is between the 4 states, without really looking for a while.

@mapk
Copy link
Contributor

mapk commented Nov 21, 2019

Initially I thought this pattern would be wonderful based on the mockups, but now that I'm interacting with it, @jasmussen's comment articulates it well.

The fact that I'm going through a two-placeholder step to get the content on the screen feels wrong. Using this pattern for Columns or Table block feel pretty natural because the user can adjust the amount of columns or rows. But in this case that can't be done.

This PR was a great exploration, @gziolo, and I appreciate the work! @gziolo, what do you think? Should we discontinue this?

@mapk mapk removed the Needs Design Feedback Needs general design feedback. label Nov 21, 2019
@gziolo
Copy link
Member Author

gziolo commented Nov 21, 2019

This PR was a great exploration, @gziolo, and I appreciate the work! @gziolo, what do you think? Should we discontinue this?

It was the purpose of this PR to explore how it would feel from the user's perspective 👍

I totally get your point and share the same sentiment that it looks nice but it doesn't make it much easier to set up the block. In fact, as discussed, you could achieve the same by finding one button in the toolbar to achieve similar result.

I'll move some parts of this PR to #18283. I think the proposed refactor to the Media & Text block can be applied to Columns block as well. I'm really happy how it shapes up and gives us a chance to find an approach where registering patterns for a block type will automatically adjust UI of the block without the need to code anything else by the block developer.

@gziolo gziolo closed this Nov 21, 2019
@gziolo gziolo deleted the update/media-text-patterns-api branch November 21, 2019 10:03
@youknowriad
Copy link
Contributor

I think patterns could be shown in different places including placeholders, and maybe this is one of those blocks where the patterns should be shown elsewhere (switcher?)

@jasmussen
Copy link
Contributor

Based on the block library/patterns discussion I've been exploring some additional mockups that I think get very close to better solving that issue. I hope to share as soon as I possibly can.

That's also a suggestion that my personal spidey sense suggests a placeholder/setup state isn't a great place to surface patterns. We've seen with the Image block and its setup state that when you show it in extremely narrow contexts, there just isn't space to show that much. I hope to look at and polish master...jameslnewell:element-queries as soon as I can, which actually allows an Image placeholder to scale to those contexts.

But this is to suggest that in order to have the space to properly see a "pattern", you likely need a popover or dialog.

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. [Package] Block library /packages/block-library [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants