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

BlockPatternsList with DataViews for inserter #68030

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

ntsekouras
Copy link
Contributor

What?

Related: #66549

This PR explores having a BlockPatternsList component powered by DataViews. This is a WIP and the PR aims to examine whether that would be a good fit design wise and what other challenges exist with the current DataViews components and the intended usage.

For now I've hacked (incompletely - but enough to get going) the patterns list in the inserter.

Would the inserter be better design wise with DataViews (filters, etc..) ? --cc @WordPress/gutenberg-design

Notes

  1. Currently grid layout implements an automatic handling of preview sizes based on the viewport and it's obvious in this PR that this can't work well when we want to have a DataViews instance in a specific container and not full screen.
  2. Keyboard navigation in grid layout is rough and we need to handle that there.

Testing Instructions

  1. Open the inserter and go to patterns tab and explore. Noting that you'll have to make your viewport a bit narrower due to note 1 above.

@ntsekouras ntsekouras added [Type] Code Quality Issues or PRs that relate to code quality [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Dec 16, 2024
@ntsekouras ntsekouras self-assigned this Dec 16, 2024
@Mamaduka
Copy link
Member

Probably related #39308.

@jameskoster
Copy link
Contributor

jameskoster commented Dec 16, 2024

Thanks for putting this together, it's cool to see that everything works :) Though I noticed that some patterns show the title beneath the preview, while others show it only in a tooltip. Also that some titles include an icon where others do not.

Do you think it would make sense to try to replicate the current experience as a first step?

If so we should:

  1. Fix the SearchControl appearance (arguably blocked by SearchControl style variants #65323).
  2. Remove the 'Preview size' view option, and force each preview to 'hug' its contents in terms of height
  3. Display patterns in a single-column format.
  4. Remove the 'Source' property – we don't have this in the pattern management page.
  5. 'Sync status' should be a 'badge field'.
  6. Make a decision on the 'Title' property; permanently visible, permanently hidden, or toggle-able?
  7. If the titles are visible, apply consistent treatment; no icons.
  8. Be consistent with tooltips; show them only when the title property is hidden.

Depending on how far we want to take this, we could improve the experience a lot (imo) by adjusting the layout a bit when patterns are visible, e.g.:

patterns

By increasing the width of the patterns panel there may even be scope to re-introduce the preview size option for single or two-column layouts.

We still need to do something about filter chips in narrow spaces, but that's a separate issue (#59696).

@ntsekouras
Copy link
Contributor Author

Do you think it would make sense to try to replicate the current experience as a first step?

Personally I'd love if we had the same treatment for all of them and control the display of title through the config dialog. It will simplify things a bit and remove any possible user confusion like why some patterns have titles and other not, etc..

If we're up to try that and not stick to the current implementation about the icon and conditional tooltip, it would be my preference.

Remove the 'Preview size' view option, and force each preview to 'hug' its contents in terms of height

What do you mean by hug? Regarding the Preview size view option, I think it will be enough if we make the layout check the current container width and not the whole viewport size. In that case we wouldn't have to make specific adjustments here and the component would work better in other screens like explore patterns.

Remove the 'Source' property – we don't have this in the pattern management page.
We need this for the filtering and I'll see if there is a way around it to just use it for filters. Having said that, in patterns page we don't show pattern directory patterns as it's about management and here is about insertion. So probably it's fine to have this field in this component.

@jasmussen
Copy link
Contributor

This is a tricky one, as there are some considerations beyond the display of list items, for example the overall width of the inserter showing both categories and patterns is very big. This is reduced in Jay's mockup, which is a win. I also tried accomplishing the same in some mockups here. It's not clear any of them are solid, though. But perhaps it's a net win?

To that end, it may be a bit of a dev-question. If Jay's mockup is implementable without too much complexity, then perhaps that's fine.

@ntsekouras
Copy link
Contributor Author

ntsekouras commented Dec 17, 2024

To that end, it may be a bit of a dev-question. If Jay's mockup is implementable without too much complexity, then perhaps that's fine.

I'm not sure without trying about the header expanding in both panels (from Jay's mockup). Besides that, do you suggest that the inserter's first panel width is reduced for every category (blocks, patterns, media) or just patterns. I'd expect to be reduced for everything.

@jameskoster
Copy link
Contributor

What do you mean by hug?

Currently I see this:

Screenshot 2024-12-17 at 13 11 25

Note how the preview container is much taller than the pattern leading to a lot of wasted space. We should improve this if possible.

do you suggest that the inserter's first panel width is reduced for every category (blocks, patterns, media) or just patterns.

The idea was that the entire inserter width expanded when you select Patterns. No more separate columns. The categories would be a vertical Tabs instance with 'All' initially selected, and the DataViews component lives in the tab panel associated with each category.

@ntsekouras ntsekouras force-pushed the try/patterns-list-with-dataviews branch from aa44a80 to c9fdc5a Compare December 23, 2024 07:58
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
None yet
Development

Successfully merging this pull request may close these issues.

4 participants