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

Some new issues regarding Flex component #4617

Open
snorrekim opened this issue Feb 25, 2025 · 0 comments
Open

Some new issues regarding Flex component #4617

snorrekim opened this issue Feb 25, 2025 · 0 comments

Comments

@snorrekim
Copy link
Contributor

Some new issues I've found regarding:

  • left right spacing
  • dividers of items
  • right spacing on container
  • wrapping children

Left and right spacing on items

On horizontal layout:
If you add left spacing to a child it is applied to both left and right
If you add right spacing it is also applied to the next element

Dividers only really work for vertical layout

The code, tests and examples suggests that we think it should work on horizontal, but it really doesn't. Is this a mistake? As I see it, it should only work on vertical, or it should change the direction to vertical.

The Flex.Container breaks if given right spacing

On horizontal, Flex.Container looses its negative right margin if given a right spacing. This happens for example, if you place a Flex.Container inside another one. There are some possible solutions. Though since there are so many ways to set space, we should rely on the Space component. If the Space component made a css variable for each space it adds, we can use that to correct.

Wrapping children

We handle fragments and _supportsSpacingProps='children' sometimes the same and sometimes different. Sometimes it creates a new flex.container, and sometimes it adds the wrapper to each child

This applies the Wrapper around every Item:

<Flex.Container>
  <>
    <Wrapper>
      <Item>1</Item>
      <Item>2</Item>
      <Item>3</Item>
    </Wrapper>
  </>
</Flex.Container>

This puts the items inside another Flex.Container inside the Wrapper

<Flex.Container>
    <Wrapper>
      <Item>1</Item>
      <Item>2</Item>
      <Item>3</Item>
    </Wrapper>
</Flex.Container>

there are also a lot of other weird behaviour.

Should we not be more clear on exactly what a fragment does vs a _supportsSpacingProps='children'? Fragments should always just be ignored. But what exactly should _supportsSpacingProps='children' do? And it's use case in vertical is different than in horizontal? We should have a clear definition so we can make the code work. Because right now it's doing a lot of unpredictable things.

theres also issues with inheriting spacing form parents vs itself. Itself should override, but sometimes it doesn't because of the wrapper logic.

Wrapper with _supportsSpacingProps='children' works differently for vertical than horizontal. On horizontal, it can act as if the children are just part of the flex layout. But on horizontal it just creates a new Flex.Container that (buggily) inherits the gap of the parent Flex.Container. You might as well just use Flex.Container with the same gap.

It also does something different depending on what level it is on.

  • On the first level, it wraps and creates a new Flex.Container with its children (space-wrapped) inside
  • On the second level (in a fragment) it wraps each of it's children (space-wrapped). This is the oddest.

Or said differently:

  • On the first level, a fragment is ignored, and a Wrapper creates a new Flex.Container
  • On the second level, a fragment is treated as a Wrapper. But since a fragment

We need to define:

  • What is the purpose of the Wrapper?
  • Should it wrap all the children, or each of the children? Not either depending on level like now.

Originally posted by @snorrekim in #4496 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant