Group block: Add a row variation #34535
Conversation
youknowriad
added
[Block] Group
youknowriad
requested review from
mtias,
oandregal,
jasmussen and
ntsekouras
|
Size Change: +238 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
add/row-group-variation
branch
from
22a5230
to
91a0f8e
Compare
|
I mean, this is working pretty well and feels immediately useful:
I guess you can transform from a group to a block, and that might be natural too. The row is also a good name, and shows up searching both row and group in the slash command:
Rather than making it a separate block/variation, I always assumed we'd want to make it a directional toggle on the group block, a la Figma auto layout:
I don't have any aversion to it being a variation, I suppose if we were to land on a similar UI as the above, it could still transform between groups and rows. A benefit of doing it this way is that this PR is tiny, and it works right out of the box. Thank you as always, Riad! Curious what others think. |
It seems having both might be good, we'd need some design explorations for the layout type switcher see #33687 |
|
This works great for themes with |
I agree, with the idea that a future control like what @jasmussen highlighted could switch between the two. That'd be huge.
I ran into this as well. Question about the variation: Will users expect to see the variation name in the List view, or the Settings sidebar? Right now, you add a "Row" but as a user I'll not see "Row" again after this point. |
I noticed this as well, I think ideally they'd see the variation somehow. (I'll try to solve that like we did for embeds) |
This will just require setting |
It could be also an array of the content of the attribute is isActive: [ 'layout' ],By the way, it's great to see how little code you need to improve the user experience 😄 |
|
I added the isActive function to show "Row" as block name... Do we want to land this as is initially or wait for the layout switcher UI first? |
The row feels sensible. What kind of deprecations would we have to do to remove the block, in case we decided horizontal was just a property of the Group block after all? |
None, that's just a regular group block, the "Row" block as a variation is just UI flavor. |
This still needs the handling for themes with no |
I think this is solved by your PR right? So we just need to wait for it to land :) |
No, it needs changes in |
|
@ntsekouras right, forgot about this check :) thanks |
youknowriad
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
youknowriad
force-pushed
the
add/row-group-variation
branch
from
6743dfb
to
7c8c9cc
Compare
|
@ntsekouras it should be fixed now. |
|
Taking a closer look at the new Row "block". I am testing Gutenberg 11.5 RC. https://github.com/WordPress/gutenberg/releases/tag/v11.5.0-rc.1 Example of adding a paragraph block:
Notice that the inserter is on top of the last letters of block. Inserting a few paragraph blocks. (first to are inside a row, and the last is a default top parent paragraph block. I inserted a Heading, a spacer (here it would be helpful to create space horizontally in addition to vertically), and a paragraph block. I clicked the Borders style and expected to see a border around the row. This is what I saw.
I did not expect to have content moved, only to have a border and the content stay in the same place. I assume that it is the spacer block creating a nuisance with the row block. Adding a /row inserting a Paragraph and writing some text. Then looking for the Inserter to add another block to the row. Row-block-missing-inserter.mp4The issue:
At this moment.... Testing perhaps something that can be useful. Above. That was this nights test. |
* trunk: (214 commits) Fix snackbar overflow on nav editor (#34661) Mobile - Allow disabling text and background color via theme.json (#34633) Fix disabled blocks logical error on Widgets screen (#34634) [Mobile] - Global styles - Add support to render font sizes and line height (#34144) ESLint Plugin: Update eslint jsdoc dependency (#34338) Scripts: Add CHANGELOG entry for `jest-dev-server` upgrade (#34657) Bump jest-dev-server to v5 (#34560) Refactor the `core-data` store to thunks (#28389) Only capture toolbars on parent Nav block when not in vertical mode (#34615) Update Changelog for 11.5.0-rc.1 Bump plugin version to 11.5.0-rc.1 Gallery block: Fix media placeholder height in site editor (#34629) Border Controls: Display color indicator and check selected color (#34467) Remove horizontal and vertical navigation block variations from inserter (#34614) AlignmentMatrixControl : Fix/update docs (#34624) Gap block support: force gap change to cause the block to re-render (fix Safari issue) (#34567) [Block Library - Social Links]: Use the new `flex` layout (#34493) [Mobile] Update the bottom sheet header (#34309) Group block: Add a row variation (#34535) Migrate entities.js to thunks (#34582) ...
|
There seems to be a bit of a major bug with Rows... they don't add any CSS classes to the rendered markup, so it's impossible to properly style Rows on the front-end. |
|
Actually, upon further inspection, there is actually a CSS class output on the rendered markup which provides the desired styles: |
|
The classes for "layouts" (whether for this block or another) are generated by the backend and not meant to be customized as structural. the classnames are unique because each block (with its own layout) can have different layout configs. |
|
@youknowriad Fair enough. I did some further investigation, and it turns out there IS a front-end styling issue with the Row... but it only affects themes without |
|
Just tested this and wanted to add my two cents:
Why is it applying the |
Follow-up #33687
Right now, the Flex layout is there but not exposed anywhere in the UI, this PR adds a first block using the new layout. It's just a variation of the group block called "row". What do you think?
Testing instructions