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

Consolidate BlockPatternsList component usages #66549

Open
ntsekouras opened this issue Oct 29, 2024 · 8 comments
Open

Consolidate BlockPatternsList component usages #66549

ntsekouras opened this issue Oct 29, 2024 · 8 comments
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Code Quality Issues or PRs that relate to code quality

Comments

@ntsekouras
Copy link
Contributor

ntsekouras commented Oct 29, 2024

Currently we render BlockPatternsList component in multiple places, but many of them have subtle differences, so it might be good to consolidate these designs and make the UI more consistent.

We can probably say we have two basic usages of this component:

  1. Render patterns in a more narrow context like sidebars.
  2. Render patterns in dialogs for replace flows - usually from a toolbar button

In 'sidebars' we render them as a vertical list and the current usages are:

Usage Screenshot
Inserter patterns tabs
Design panel for replacing template parts and templates
There is an exception of the sidebar cases in the search results of the inserter, where we render the results as a two columns grid.

In dialogs:

Usage Screenshot
Change design for patterns in zoom out
Replace for Query Loop patterns. This toolbar item opens a full screen modal with 3 columns in wide screens and 2 in smaller ones and also renders the patterns in a masonry style with column-count css and not grid.
Explore all patterns render in a full screen modal with 3 columns in wide screens and 2 in smaller ones - uses grid css.
Swap template for pages under template dropdown render in a full screen modal with 2 two 4 columns based on viewport- uses column-count css. Similar usages with this one are: Choosing a starting template when creating a new page or a template.

Need feedback

  1. In all of our 'grid' implementations we use adhoc styles per use case. Some of them are identical, some are not (number of columns, masonry style or not, etc..). It makes sense to absorb this grid usage in the component itself with a new variant or something similar. In order to do that we need to decide what the final design for all them would be.
  2. Should the dialog implementations be considered as a third basic usage? Maybe the dialogs when creating new entities should remain in full screen dialogs, but the replace in Query Loop should probably be consolidated with the rest replace flows.
  3. We have the concept of replace and some times we render it in toolbar and some times in inspector controls. Should we consolidate them or it makes sense as is?

I'd love to hear some feedback from @WordPress/gutenberg-design , --cc @richtabor @jameskoster @jasmussen .

@ntsekouras ntsekouras added [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Code Quality Issues or PRs that relate to code quality labels Oct 29, 2024
@jameskoster
Copy link
Contributor

Render patterns in dialogs for replace flows

Aside from the 'Change design' example (which arguably feels more closely related to the sidebar placements) have we considered making these data views (in grid layout)? If so the task can be as simple as defining the data view config :)

There may even base an argument for making the other examples data views too. Then we get search, filtering, and view options like grid density 'for free'. Just an idea.

@ntsekouras
Copy link
Contributor Author

ntsekouras commented Oct 30, 2024

have we considered making these data views (in grid layout)? If so the task can be as simple as defining the data view config :)

In some cases like the inserter or pattern explorer this could make sense, but I'm not sure it's needed for the rest.

Having said that, we still need to consolidate all these usages.

@jameskoster
Copy link
Contributor

Totally agreed. Let's define which scenarios can use data views, then we can set about designing a component for the remaining ones.

Like you said, Inserter and Pattern explorer make sense. Maybe 'Replace' in Query Loops and 'Swap template' too?

@t-hamano
Copy link
Contributor

t-hamano commented Nov 26, 2024

I came here from #66549 #67243. This issue seems to be mostly about layout, but is it possible to discuss whether to show visual titles as well?

@ntsekouras
Copy link
Contributor Author

I came here from #66549. This issue seems to be mostly about layout, but is it possible to discuss whether to show visual titles as well?

Of course. I think that's part of it too.

@youknowriad
Copy link
Contributor

I've been made aware of some memory issues when the list of patterns becomes high. For instance in the swap template modal. I think we should give some priority to this consolidation work and also consider using DataViews internally within the component to address things like: pagination and search (solves the memory issues), grid configuration...

@ntsekouras
Copy link
Contributor Author

I think we should give some priority to this consolidation work and also consider using DataViews internally within the component to address things like: pagination and search (solves the memory issues), grid configuration...

I have this in mind and this consolidation also relates to work for moving the pattern fields (example here) in order to use the DataViews patterns internally, outside the edit-site package.

@youknowriad
Copy link
Contributor

I have this in mind and this consolidation also relates to work for moving the pattern fields (example #67449) in order to use the DataViews patterns internally, outside the edit-site package.

It's not really clear to me that this should use these shared fields. The shared fields in @wordpress/fields are about the entities (The pattern entity). The "pattern" objects received by this component are not necessarily "Pattern" entities. They can be just temporary objects created from templates or patterns or any random list of blocks. So IMO, it might not be a good idea to use the @wordpress/fields package there, in fact it's not possible to use it at all because @wordpress/fields depends on core-data and block-editor can't.

So instead, if we're going to use DataViews here, we should just declare the fields locally within the BlockPatternsList component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Code Quality Issues or PRs that relate to code quality
Projects
Status: Now
Development

No branches or pull requests

4 participants