Why do our Stack components not support more flex properties?

[sophschneider]

Gathering context on why we do not support more flex properties in the system.

We used to have a <LegacyStack.Item fill >. Now this has to be achieved using css grid via our InlineGrid component <InlineGrid columns={{ xs: "1fr auto" }}>. Some builders have brought up that they still want to use flex instead of grid.

Questions to discuss:

1. Both LegacyStack and our current Stack components have only ever supported a subset of flex properties. What is the historical context for doing this?
2. What would be some use cases (examples in the admin and sandboxes would be helpful) for the system to support flex properties? what are the trade offs of doing this?
3. If we were to add more flex properties into our system, would we add it onto Box, extend ourStack components (would have to support child wrappers), or create a new component?
4. Could we just suggest that builders use custom css for complex layouts that cannot be solved by our current components?

[jesstelford]

1 & 2.

What is the historical context for doing this? [..] What are the trade offs of doing this?

Every property we add to <Box> increases the size of the generated CSS file due to how we're supporting responsive style props, so we've been hesitant to add more properties.

would we add it onto Box, extend ourStack components (would have to support child wrappers), or create a new component?

Box is where it should live, then it should be exposed as a prop on Stack which passes it down to Box.

suggest that builders use custom css

Custom CSS adds to the burden of doing Uplift-like projects: migrating is tricky to automate and custom styles can very quickly conflict with new changes in Polaris as we ship even minor releases. IMO the CSS of Polaris should be considered a private API and never overwritten or relied upon by consumers. But that stance requires a better, more flexible alternative which meets our needs as library maintainers and their needs for flexibility as builders.

[jas7457]

@jesstelford for #1, are you sure we'd want that to add that to Box? Properties such as flexGrow and flexShrink (or maybe we call it grow and shrink) don't make sense outside of the context of the item being a flex child. Box is meant to be generic, and context unaware. I'd be hesitant to add contextual properties to something like this.

Instead, I'd probably rather want to see a VerticalStack.Item and HorizontalStack.Item (that are actually the same component under the hood) that has some flex children specific properties like grow, shrink, alignSelf, etc. This might just be shifting the problem if we decide these need to be breakpoint specific, but as a consumer of Polaris/FE dev in general, it's a lot less likely to need these type of properties to be breakpoint specific.

[sophschneider]

I wonder if there's a solution where we would keep everything on the Stack parent instead of introducing children wrappers to avoid composition mistakes. For example, a prop that maps flexGrow and flexShrink to its immediate children:

For example, <InlineStack flexItems={['1 0 0', '0']}> would map each flex shorthand to each of the immediate children

[jas7457]

to avoid composition mistakes

Can you elaborate what you mean exactly?

For example, <InlineStack flexItems={['1 0 0', '0']}>

Cool idea, but I'm hesitant to use this approach. A lot of the time, I have children which are conditionally rendered, or swapped depending on some condition. To replicate this at the parent Stack layer and the children layer seems like it could very easily lead to (potentially subtle, hard to reproduce) bugs.

[jesstelford]

don't make sense outside of the context of the item being a flex child.

Fair enough, and that makes sense in a world where we only expose flexGrow, etc, on <Box>. In my head I am imagining that <Box> would expose all CSS properties unapologetically to provide the most flexbility for our builders.

Then, higher-level / more context-aware components such as Stack can reduce the number of CSS properties that are exposed to a subset that makes sense in that context. The lack of this type of escape hatch is what leads to these incoming questions over and over again, then folks eventually reach for custom CSS with all its trappings as I mentioned above.

[patrickracicot]

The lack of this type of escape hatch is what leads to these incoming questions over and over again

I think its also very important to note that for the flex-grow like approach the question comes even more often due to the fact that this layout could be achieve with native Polaris components, but in the new offering we no longer have the feature parity and with LegacyStack now being deprecated, teams have a fear of needed a bunch of custom CSS to keep their layout working.

[aveline]

Initially the thinking was that Box was a way to access token values within the system for things like borders, background colors, etc. and that layout should be handled separately with layout components. This is why we have hesitated to add layout related properties to Box. Even though it can create more nesting to have a layout component to handle spacing and layout and a Box component handling more visual styles, it forces separation of concerns.

We have held off adding flex to Stack because we wanted to keep the initial layout components very simple and avoid introducing sub components to manage those flex specific child properties.

Seems like we've hit a point where we are seeing need for cases beyond what the initial simple layout components handle, so thank you @sophschneider for getting this discussion going!

[aveline]

A few more thoughts, InlineStack usage is currently about 1424 and BlockStack is 2242

When we created that first set of layout primitives they were meant to be small, single purpose components. They were not meant to handle every possible layout case.

Based on the usage of those two components, it seems they useful for what they do well. What they don't do well is handle cases that flex would.

So I do question whether to extend the stack components, or create a new component to handle the types of things folks are wanting to accomplish with flex. And perhaps having a better understanding of exactly what those things are would also be helpful.

[patrickracicot]

A quick way to see where flex stack would be useful is looking at our codebase in Shopify for all the current projects still using <LegacyStack.Item fill>

[jas7457]

Another good data point would be looking at code that uses Horizontal or VerticalStack with some type of div/span as a direct child (signifying they likely had to add a className or do something else to get the layout they wanted): https://github.com/search?q=org%3AShopify+%2F%28VerticalStack%7CHorizontalStack%29.*%3F%5Cn.*%3F%3C%28div%7Cspan%29%2F&type=code