Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

[Atomic Block] Support for Columns/Column #2526

Closed
2 tasks
mikejolley opened this issue May 20, 2020 · 8 comments
Closed
2 tasks

[Atomic Block] Support for Columns/Column #2526

mikejolley opened this issue May 20, 2020 · 8 comments
Labels
focus: components Work that introduces new or updates existing components.

Comments

@mikejolley
Copy link
Member

Core provides core/columns and core/column blocks which we could use in the Single Products Block to lay out the content as mocked up. The challenge will be supporting this in our LayoutConfig so that they work on the frontend.

Note, we cannot rely on saved InnerBlock content because it is not dynamic.

  • Find a method to render columns/column on the frontend via LayoutConfig
  • Columns use Inner Blocks but the blocks are not limited. We need to find a way to limit what blocks can be inserted in this area.

If this is not possible, we will need to create a custom column block.

@mikejolley mikejolley added the focus: components Work that introduces new or updates existing components. label May 20, 2020
@mikejolley
Copy link
Member Author

mikejolley commented Jun 3, 2020

I've hit some roadblocks using the columns block; whilst it renders fine, we're not able to control which Inner Blocks are allowed within a column. This also restricts us from enabling the Atomic blocks for columns since we don't want them appearing in other column contexts outside of the products block.

We could duplicate columns functionality within our plugin, however, I only want to do this if we're set on using columns for achieve this layout... So this is a question for @LevinMedia - in the mocks, single product block uses a 2 column layout.

  • Is this something we want to fix at 2 columns?
  • Do we want controls at column level (column width etc)?
  • Do we need multiple "rows" or columns added i.e. support multiple "columns" blocks?

I have tinkered with adding a single custom "column" block and having the single block act as the container (equivilent to columns block), which may work but it depends on our needs.

Once I gain understanding on what we actually need for these columns I can more easily make a call regarding what needs development.

cc @nerrad

@mikejolley
Copy link
Member Author

So had a quick chat with @LevinMedia, it sounds like we're probably going to need a columns like interface regardless (noting that a block can only have one InnerBlocks child), so options would be to:

  1. Copy columns/column from Gutenberg into blocks and have some custom versions which allow us to limit allowedBlocks as needed for our use case, or
  2. Remove all restrictions on allowedBlocks for now (that also means allowing the atomic blocks to be inserted into any column, even if non-functional) and then try to get a PR merged to Gutenberg to allow us to limit them in future.

@nerrad do you have any thoughts on this? If we went with (2) we'd need something like this perhaps you can assist with getting something approved WordPress/gutenberg#18164

@nerrad
Copy link
Contributor

nerrad commented Jun 3, 2020

I'll give a more thorough look at this at a later point, but one thing I noticed is that the allowed blocks you linked to doesn't look like it's directly related here because according to this, the only allowed block is core/column. So it looks like there may still be some digging needed to find out how any block is added in a column.

Generally, I think we want to make absolutely sure we have to rule out using the core/columns block (including checking with the Gutenberg team) if needed.

Addendum: (your most recent comment came in as I was typing this)

If needed, I think we should contribute to Gutenberg to get something usable for our needs as this looks like it will help with other needs in the project (and other projects) as well.

I realize this does mean we'd still have to implement a workaround (uncertain about 2 as an option yet... I'm concerned about back-compat on existing blocks if later we add restrictions) if we want to support older versions of WP core (until any contributions we make upstream land in something matching our minimum requirements) - or alternatively we might be able to directly import any contributed changes (assuming there's tree-shaking implemented for the package we import from).

@mikejolley
Copy link
Member Author

mikejolley commented Jun 3, 2020

the only allowed block is core/column

@nerrad Thats for columns block. That restriction is ok. It's column (singular) where we need to define allowedBlocks https://github.com/WordPress/gutenberg/blob/245e21befe1c2dfc69334b3aa06578ea93ad45da/packages/block-library/src/column/edit.js#L62-L73 I used the wrong link.

@mikejolley
Copy link
Member Author

@mikejolley
Copy link
Member Author

mikejolley commented Jun 4, 2020

I don't know if this would be best practice or not @nerrad but it seems to me Gutenberg could benefit from some context to control trees of InnerBlocks.

So lets say we had an InnerBlockContext which accepted a prop named allowedBlocks, all InnerBlocks within that context would inherit the context's allowedBlocks property, unless they defined it explicitely.

That would inherently make columns useful for us again, because column does not define allowedBlocks at all.

Basically, make the parent/context allowedBlocks property apply to all InnerBlocks within the tree.

Do you think something like that would be accepted, or do you think it's more likely to need something like WordPress/gutenberg#18164?

@nerrad
Copy link
Contributor

nerrad commented Jun 4, 2020

Do you think something like that would be accepted, or do you think it's more likely to need something like WordPress/gutenberg#18164?

I think context would be accepted over attributes for all the reasons Jorge outlined in his feedback on 18164. That's the approach I'd try taking initially anyways.

The only issue I could forsee is that while context might be how it's implemented in the end, the provider itself might be absorbed by some other mechanism so plugins don't have to use it directly (even though I think that might be the most explicit way to do it).

Another advantage with context, is feasibly it could be something that's opted in to via more core blocks as we become comfortable with the api (and potentially other things could be exposed via the context).

Definitely something worth exploring for Gutenberg anyways and I'd be happy to collaborate with getting something landed 👍

@mikejolley
Copy link
Member Author

Due to recent conversations this is no longer a problem; columns will be supported due to restrictions being removed from Atomic Blocks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: components Work that introduces new or updates existing components.
Projects
None yet
Development

No branches or pull requests

2 participants