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

Make it possible to set the width of blocks within Row #44467

Closed
jameskoster opened this issue Sep 26, 2022 · 11 comments · Fixed by #45364
Closed

Make it possible to set the width of blocks within Row #44467

jameskoster opened this issue Sep 26, 2022 · 11 comments · Fixed by #45364
Assignees
Labels
[Block] Group Affects the Group Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Layout Layout block support, its UI controls, and style output. Needs Dev Ready for, and needs developer efforts

Comments

@jameskoster
Copy link
Contributor

jameskoster commented Sep 26, 2022

A small part of #42385. Regardless of the Justification setting on the parent, child blocks within a row will always "hug" their content (except for blocks like Image that can have a fixed width applied).

Screenshot 2022-09-26 at 14 07 24

If it were possible to apply different width rules to child blocks then the creation of more elaborate layouts would be simplified.

Screenshot 2022-09-26 at 14 29 41

Additional options to consider:

  • Fixed – same as Images, drag-handles to resize. In the future this should ideally be informed by an underlying grid.
  • Fill – The block will stretch to occupy all remaining space in the container.

Here's how it could look:

fill, hug, fixed control

  • Layout panel becomes a ToolsPanel, allowing it to add additional control, in this case "Width".
  • A segmented control defaults to "Fill", offers options for "Hug" and "Fixed" (see also conversation)
  • If we were to add a drag handle in the canvas, pulling such a handle would automatically set the toggle to "Fixed".

Issue updated Sep 30.

@jameskoster jameskoster added Needs Design Needs design efforts. [Block] Group Affects the Group Block labels Sep 26, 2022
@jasmussen

This comment was marked as resolved.

@jameskoster

This comment was marked as resolved.

@jasmussen

This comment was marked as resolved.

@jameskoster

This comment was marked as resolved.

@jasmussen
Copy link
Contributor

Looks good!

(Nit: hould use the 40px heights, padding and border color in input looks wrong, as does the padding on the segmented control.)

What happens if I set all child blocks in a container to 100px, but the overall container is only 200px wide? Should there be some logic to restrict the values?

We respect what would happen if you wrote this CSS. It breaks out of the boundaries. CSS is awesome.

Hug seems a little tricky to interpret.

It does, yes, but it's a way shorter word than "constrain width to content", and even "content width", and the help text helps.

Hug works well for images, but for text... what happens to a single Paragraph in a row that is set to hug? Does it overflow the container or wrap? If it wraps then in this case hug and fill are the same.

Seems worth moving this into a codepen, and writing the accompanying CSS to try this out in practice, seeing what happens to short and long paragraphs, with fill and hug and fixed, etc.

Keep in mind, both the container and the child can have hug/fill/fixed options. I imagine a fixed width container, with overflow hidden (still missing that in the Advanced section) and a long paragraph set to hug inside, would be cropped off. This can make sense for future eliding options.

Do we need to consider overflow on the parent container?

Not for this ticket, but yes I think there's a separate ticket for it, would be nice to have that in advanced.

@jameskoster
Copy link
Contributor Author

Nit: hould use the 40px heights

We really need to update our Figma components :D

Here's a quick codepen I made to explore the options. Observations:

I was surprised to see that children with Fill width don't occupy equal horizontal space by default. A % width value had to be added based on the number of items in the row. This is a bit of a shame as it means we'd need to do some dynamic calculations based on the number of children. Hopefully this isn't a big deal but could get tricky if/when we account for wrapping?

The only way I could get Hug to work was to set a fixed width based on the total width of the children. This makes me wonder if this option should be reserved only for containers. Which leads to a follow-up question... if you add the width tool to a paragraph, what is the default setting, Fill?

We need to account for conceptually incompatible configurations. See demos "Row with Hug width and Fill width children" and "Row with Hug width and Fixed and Fill width children" for examples. Neither are behaving as expected. As I'm sure you know, Figma doesn't allow these configurations – if you attempt to create one then it automatically updates the width on the parent or child respectively to create a compatible config (and displays a Snackbar to inform you of what's going on). We'll probably need to do something similar.

@tellthemachines
Copy link
Contributor

Oooh this is an interesting idea! We'll need some sort of mechanism to add the child Layout panel to all the child blocks of a block with flex layout. This would potentially extend further than Row, and into blocks such as Buttons, Social Links and Navigation. I guess with ToolsPanel hiding the child controls by default, that won't be an issue?

What happens if I set all child blocks in a container to 100px, but the overall container is only 200px wide? Should there be some logic to restrict the values?

We respect what would happen if you wrote this CSS. It breaks out of the boundaries. CSS is awesome.

I couldn't resist having a play with the codepen 😄 the only issue I can see is in content being pushed out beyond the viewport if combined widths overflow it. We'd have to use flex-basis to actually fix the width:
Screen Shot 2022-09-30 at 11 54 55 am
(gold background shows width of the flex container)

I was surprised to see that children with Fill width don't occupy equal horizontal space by default. A % width value had to be added based on the number of items in the row. This is a bit of a shame as it means we'd need to do some dynamic calculations based on the number of children. Hopefully this isn't a big deal but could get tricky if/when we account for wrapping?

We can try flex-basis: fit-content on the children here, it should work for any number of items:

Screen Shot 2022-09-30 at 11 58 09 am

@tellthemachines tellthemachines added the [Feature] Layout Layout block support, its UI controls, and style output. label Sep 30, 2022
@andrewserong
Copy link
Contributor

andrewserong commented Sep 30, 2022

This would potentially extend further than Row, and into blocks such as Buttons, Social Links and Navigation.

It could also be very cool for both the Gallery block (to have control over column span of individual images, and replace the ad hoc width implementation), and also fine-grained control over columns in the Post Template block, too (related issue to refactor Post Template block to use the flex layout type properly: #44557)

@jasmussen jasmussen added Needs Dev Ready for, and needs developer efforts and removed Needs Design Needs design efforts. labels Sep 30, 2022
@jasmussen
Copy link
Contributor

Because we have a design that should provide a great starting point, I've updated this ticket and added the "Needs dev" label!

@tellthemachines
Copy link
Contributor

I've updated #45364 and it's now ready for review!

One thing regarding child blocks with "Fill" set not occupying an equal amount of horizontal space: this is impossible to deal with on the child level, because it depends on what type and size the sibling blocks are. If we do want to have a way to set equal width on all children of a Row block, I'd suggest setting it as a control on the actual Row layout. It could just be a toggle, and it would add a rule like .wp-container-35 > * { flex: 1 1 0;}, although if any of the children have a flex-basis set we'll have to decide if we'd want to override that at the parent level (by setting !important! on the above rule) or not. What do you all think?

@jasmussen
Copy link
Contributor

jasmussen commented Nov 28, 2022

Thanks for all the work here, this is exciting.

Designwise, I think we can potentially simplify this even further, @jameskoster and I will follow up with some designs we can iterate in.

One aspect that was discussed a great deal in #42385 was the responsive behavior, of which fit and fill both handle things well. However, we forgot how to account for "Fixed" in this, so it may be good to follow up on this pretty quickly so we start with expectations of smart defaults. That is, if I set a menu item to be 400px wide, this really only makes sense to do in a non-responsive design.

I created this quick codepen to look at flex behavior for a specific width item: https://codepen.io/joen/pen/rNKrVeZ?editors=1100 — this one is working as I would expect it to, that is, 400px is a max-width but when I resize down, Item 1 is allowed to scale and doesn't push off items from the container. To put it differently, it's behaving as if it's Fill + 400px wide, which is good. That's the fixed width behavior we want for these controls as well, but that doesn't appear to be the case in trunk at the moment.

What flex arcana do we need to tinker with to make that happen? Is it flex-basis, flex-shrink, flex-grow related? Do we need to look into clamp values? CC: @tellthemachines @andrewserong @priethor

A bit more here: #45364 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Layout Layout block support, its UI controls, and style output. Needs Dev Ready for, and needs developer efforts
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants