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

Split "flex" layout type into separate "row" and "stack" layout types #44880

Open
cbirdsong opened this issue Oct 11, 2022 · 1 comment
Open
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Enhancement A suggestion for improvement.

Comments

@cbirdsong
Copy link

Picking up from #39336 (comment):

"Flex" attempts to encapsulate all "one-dimensional" layouts in a single type, but I think "one-dimensional" isn't enough to hang a type on conceptually. You see the issues start as soon as you write the justification utility classes. For vertical layout, you need to modify align-items, and for a horizontal layout you need to modify justify-content, and if your classes are human-readable they are basically unable to operate in both contexts. Look at how the buttons block CSS needed to be written1:

.is-layout-flex {
  &.is-vertical {
    flex-direction: column;
  }

  &.is-content-justification-left {
    justify-content: flex-start;
    &.is-vertical {
      align-items: flex-start;
    }
  }

  &.is-content-justification-center {
    justify-content: center;
    &.is-vertical {
      align-items: center;
    }
  }

  &.is-content-justification-right {
    justify-content: flex-end;

    &.is-vertical {
      align-items: flex-end;
    }
  }

  &.is-content-justification-space-between {
    justify-content: space-between;
  }
}

Changing layout behavior based on the presence of .is-vertical is needed at basically every step of the way, because it's an entirely different layout. Everything is much cleaner when you just start with that premise:

.is-layout-stack {
  flex-direction: column;

  &.is-content-justification-left {
    align-items: flex-start;
  }
  &.is-content-justification-center {
    align-items: center;
  }
  &.is-content-justification-right {
    align-items: flex-end;
  }
}

.is-layout-row {
  flex-direction: row;

  &.is-content-justification-left {
    justify-content: flex-start;
  }
  &.is-content-justification-center {
    justify-content: center;
  }
  &.is-content-justification-right {
    justify-content: flex-end;
  }
  &.is-content-justification-space-between {
    justify-content: space-between;
  }
}

The layout types are conceptually much cleaner when split this way, and the names "stack" and "row" map directly onto the group block variations with the same name.

Footnotes

  1. Lightly modified into a generic "flex" layout for easier comparison.

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. [Feature] Layout Layout block support, its UI controls, and style output. labels Oct 12, 2022
@tellthemachines
Copy link
Contributor

Thanks for opening this issue @cbirdsong !

For some historical context, in the initial implementation of vertical orientation for flex, it actually started out as a separate layout type. Following some discussion I moved to making it a flex variation, the main reason being to not duplicate the whole layout logic.

There are arguments to be made for either choice; to me what jumps out the most from your example above is that the second scenario requires less lines of CSS to be output for each variation (in that respect, the Navigation block approach may be a bit better)

But if we split flex layout into two different layout types, we'll be outputting a larger amount of CSS overall even if those layouts aren't both in use, because default styles for each layout type are always output, so performance-wise there's no real gain.

Ultimately, because is-layout-flex has been in use for a while, we won't be able to confidently remove it while ensuring backwards compatibility, which limits the usefulness of splitting flex into two types at this point. We should, however, take these ideas into account when exploring new layout types to be implemented, such as grid-based ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Layout Layout block support, its UI controls, and style output. [Type] Enhancement A suggestion for improvement.
3 participants