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

Slot-based horizontal alignment in HorizontalLayout #5038

Open
1 of 19 tasks
rolfsmeds opened this issue Feb 12, 2024 · 18 comments
Open
1 of 19 tasks

Slot-based horizontal alignment in HorizontalLayout #5038

rolfsmeds opened this issue Feb 12, 2024 · 18 comments
Labels
acceptance criteria used for the acceptance criteria checklist DS Design System feature (e.g. component) v24.7

Comments

@rolfsmeds
Copy link
Collaborator

rolfsmeds commented Feb 12, 2024

Description

An intuitive model for horizontally aligning elements in a HorizontalLayout, based on separate slots for left, center and right aligned elements.

Example

image
This layout has content aligned to the start, center and end:
image

With the existing API, you need to either

  • Wrap each group in it's own wrapper and set justify-content:space-between, or
  • Set margin-left/right to auto on the correct elements

The purpose of the proposed feature is to simplify layouting by eliminating the need for wrappers or manual managing of auto-margins.

Acceptance criteria

  • API should use writing-direction-agnostic names for RTL support: start, center, end
  • Implemented as two new slots (in addition to the current one): center and end.
    • Inline margins on first/last children of slots are used to achieve the alignment.
  • Flow API proposal:
    • addToStart(Component...) (equivalent to normal add(Component...))
    • addToCenter(Component...)
    • addToEnd(Component...)
    • Internally, component indexes must be managed to maintain start -> center -> end order, so that index-based APIs (indexOf, getComponentAtIndex), work intuitively
    • JustifyContentMode doesn't work together with center and end slots. This needs to be mentioned in the javadoc.
    • getComponentCount() etc will return total component count regardless of alignment slots.
  • WC & React API is based on setting slot attribute on elements

Examples

To produce
image

HorizontalLayout hl = new HorizontalLayout();
hl.addToCenter(new Span("C1"), new Span("C2");
hl.addStart(new Span("S1"), new Span("S2");
hl.addToEnd(new Span("E1"), new Span("E2");

produces the index order
S1, S2, C1, C2, E1, E2
(note that the indexes are according to slot order rather than the order in which they were added)

<vaadin-horizontal-layout>
  <span slot="center">C1</span>
  <span slot="center">C2</span>
  <span>S1</span>
  <span>S2</span>
  <span slot="end">E1</span>
  <span slot="end">E2</span>
</vaadin-horizontal-layout>

(the effective DOM-order of the elements is defined by the order of the slots in shadow DOM, so it doesn't matter that they're not declaratively in that order)

The above will render as
[S1 S2 C1 C2 E1 E2]

hl.getIndexAt(3) => C2 (note that this is despite C2 being added before S1 and S2)

hl.addComponentAtIndex(4, C3) =>
[S1 S2 C1 C2 C3 E1 E2]

General criteria

  • APIs reviewed
  • Design
  • Performance
  • UX/DX tests in Alpha
  • Documentation:
  • How to test?
  • Limitations:

Security

  • Security implications have been taken into account (elaborate or link to product security requirement specification if there are implications)
@rolfsmeds rolfsmeds added acceptance criteria used for the acceptance criteria checklist DS Design System feature (e.g. component) labels Feb 12, 2024
@rolfsmeds
Copy link
Collaborator Author

In practice, the index management probably means something like:

  • Maintaining a slot-to-component mapping that's updated every time a component is added, removed or replaced.
  • Implementing addToCenter and addToEnd internally using addComponentAtIndex to add the component to the end of the desired slot.

@rolfsmeds
Copy link
Collaborator Author

Open questions:

  • Should the default slot be start or center?
  • Should center be called middle as it's technically not in the center of the layout but in the middle between start and end?

@rolfsmeds rolfsmeds moved this to March 2025 (V24.7) in Roadmap Jan 8, 2025
@rolfsmeds rolfsmeds added this to Roadmap Jan 8, 2025
@web-padawan
Copy link
Member

Should center be called middle as it's technically not in the center of the layout but in the middle between start and end?

I guess that would make more sense since with center some people might expect it to be aligned the same in 3 cases:

  • only start and center are set,
  • only end and center are set,
  • all 3 are set,
  • only center is set.

Whereas with using margin-left: auto and margin-right: auto we would get this (here's the branch):

Screenshot 2025-01-08 at 16 38 49

@web-padawan
Copy link
Member

Should the default slot be start or center?

I guess this depends on how we want to handle Text component instances (which can't have slot attribute).
Using start probably makes more sense since if you only have Text in layout it'd be placed at the start.

@rolfsmeds
Copy link
Collaborator Author

mmh, I honestly don't think we need to consider text nodes in layouts – first of all it's extremely rare, and secondly we can simply document that this feature doesn't work with text nodes at all.

As for the behavior, I've tweaked my codepen behavior proto a bit and I think this would make a lot of sense: https://codepen.io/rsmeds/pen/dygYExp

@rolfsmeds
Copy link
Collaborator Author

(Frustrating that we don't yet have :has-slotted to target empty/non-empty slots, but apparently it's coming)

@web-padawan
Copy link
Member

I honestly don't think we need to consider text nodes in layouts – first of all it's extremely rare, and secondly we can simply document that this feature doesn't work with text nodes at all.

We would need to make sure addToEnd() throw an exception when passing Text component indeed. So maybe this could also be applied to other two methods for consistency, regardless of which one we decide to use the default slot.

Speaking of addToXX() API - what if we instead use add() and then a single method e.g. something like this:

public void setHorizontalComponentPlacement(Placement placement, Component... components) {

This would be aligned with setVerticalComponentAlignment() API, and also should presumably make it easier to toggle the component between e.g. center or end slots dynamically depending on certain conditions (e.g. window resize).

@rolfsmeds
Copy link
Collaborator Author

Yes, I do think it makes sense to have such a setter (which means also having an enum for the placement), however: there also needs to be an add API for setting the component to the desired placement directly, e.g.

add(Placement placement, Component... components)

And I wish we could find a better term than placement, but I have no idea what that would be.
The most intuitive term would of course be alignment, but that's already taken by the Aligment enum for the cross-axis alignment (I really with that would have been given a more specific name...), which of course has options that are not applicable here...

@sissbruecker
Copy link
Contributor

I wonder if and how this should work responsively. For example when using the wrap theme, you would get a behavior like this:

Unwrapped

Bildschirmfoto 2025-01-09 um 11 03 48

Last element wrapped

Bildschirmfoto 2025-01-09 um 11 04 00

Last two elements wrapped

Bildschirmfoto 2025-01-09 um 11 18 07

All of those look off. I would probably would want to wrap each group below each other first, in which case I have to add containers anyway, plus figure out some CSS to apply the wrapping at some breakpoint.

@vursen
Copy link

vursen commented Jan 10, 2025

And I wish we could find a better term than placement, but I have no idea what that would be.

Maybe an alternative to "placement" could be "slot"

@vursen
Copy link

vursen commented Jan 10, 2025

I wonder if and how this should work responsively. For example when using the wrap theme, you would get a behavior like this

Here is my version, based on Rolf's prototype, which seems to handle wrapping in a slightly more expected way: https://codepen.io/vursen-the-selector/pen/NPKyZMz

@web-padawan
Copy link
Member

web-padawan commented Jan 13, 2025

Reworked the prototype using pseudo elements acting as "spacers" to avoid overriding margin on slotted elements.
This also removes the need for state attributes e.g. last-start-child and first-end-child. Here's the change.

The only problem with this approach is that added pseudo-elements are affected by the gap when spacing is used.
This extra gap be also noticed especially when both spacing and wrap theme variants are combined.

@rolfsmeds
Copy link
Collaborator Author

I suppose the gap-doubling could be compensated for with a negative margin the size of the gap?

@rolfsmeds
Copy link
Collaborator Author

Hmm, here's a codepen poc with pseudo-elements, negative margins, and justify-content:end to force the end-slot contents to always be end-justified: https://codepen.io/rsmeds/pen/XJrqeJp?editors=1100 Seems to work quite nicely.

@rolfsmeds
Copy link
Collaborator Author

I've also tinkered with a model where start, center and end have their own wrappers, but the problem with that approach is that it prevents flex-grow from working on the items.

@web-padawan
Copy link
Member

Created two new branches with prototypes based on the above codepens:

IMO the main benefit of the pseudo elements approach is better since it doesn't interfere with user styles:

  1. No need to apply margin on slotted elements as that can be easily overridden e.g. by using margin CSS property, or e.g. by placing a nested horizontal / vertical layout with setMargin(true),
  2. No need to add / remove state attributes like first-end-child or last-start-child which are required because it's not possible to use :first-child to target first element in a specific slot (there's an issue about it here).

@web-padawan
Copy link
Member

web-padawan commented Jan 15, 2025

Here's one more problem with pseudo elements approach: at some point the element isn't wrapped but the middle child is:

Image

I guess we could workaround this by setting justify-content: center when middle slot is not empty and end slot is empty.

UPD: looks like it doesn't help in all cases. I didn't find a way to make sure the pseudo element gets wrapped with the actual slotted content. So maybe I'll go with the original idea of setting margin on the child elements after all.

@web-padawan
Copy link
Member

web-padawan commented Jan 15, 2025

Created a draft PR using margins approach: vaadin/web-components#8515

Seems like some wrapping problems are inevitable:

  1. When only "end" slot content wraps to the second like, it gets aligned to start:
Image
  1. When there is more than one element in the "end" slot, wrapping looks a bit weird:
Image

UPD: probably could be solved by adding state attributes like first-in-row on the corresponding end slot child.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acceptance criteria used for the acceptance criteria checklist DS Design System feature (e.g. component) v24.7
Projects
Status: March 2025 (24.7)
Development

No branches or pull requests

5 participants