web-components icon indicating copy to clipboard operation
web-components copied to clipboard

[grid] All rows visible grid sets align-self and "overrides" parent's "align-items"

Open stefanuebe opened this issue 2 weeks ago • 4 comments

Description

When setting a grid to "all rows visible" it gets extra stylings applied, including a align-self: flex-start. This means, that when putting such a grid into a vertical layout with something different then flex start, (e.g.align-items: stretch), the layout's align items will be overridden.

This leads to a necessary additional styling for the grid (align-self: stretch).

Expected outcome

I don't know the background of that all rows visible align-self: flex-start. IMO the all rows visible should not set align-self at all, since it takes the responsibility of aligning its children by default from the container and leads to the above mentioned additional styling, that has to be added to the theme stylings without a real reason.

Therefore ideally the align-self should be removed.

Minimal reproducible example

@Route("")
public class TestView extends VerticalLayout {
    public TestView() {
        setSizeFull();
        setAlignItems(Alignment.STRETCH);

        var grid = new Grid<Integer>();
        grid.addColumn(integer -> "Value a " + integer);
        grid.setItems(IntStream.range(0, 20).boxed().toList());
        grid.setAllRowsVisible(true);

        grid.getStyle().setBorder("2px dashed red");
        addAndExpand(grid);
    }
}

Steps to reproduce

  1. Download the starter
  2. Add the above view
  3. Remove the lumo theme (otherwise the overridden stretch will not be visible, since Lumo also sets width: 100% on an all rows grid)
  4. Open the view
  5. The grid is aligned left instead of being stretched

Environment

Vaadin version(s): 25 rc1

Browsers

No response

stefanuebe avatar Dec 14 '25 12:12 stefanuebe

Potential fix (in grid base styles), if we’re willing to risk a minor breaking change in behavior (as this has been working like this for 7 years):

:host([all-rows-visible]) {
+   height: max-content;
-   height: auto;
+   align-self: initial;
-   align-self: flex-start;
    min-height: auto;
    flex-grow: 0;
    flex-shrink: 0;
}

jouni avatar Dec 15 '25 08:12 jouni

For context:

  • The Grid has align-self:stretch by default, so this is not just all-rows-visible specific.
  • all-rows-visible mode changes it to flex-start
  • As mentioned above, that was added 7 years ago (PR) to make the Grid "shrink-wrap" its height to its rows (rather than stretching beyond that by default).
  • We could remove both align-selfs, but as there have been zero tickets about this (that I'm aware of) in the past 7 years, I'm not sure it's worth introducing a breaking change to address it.
  • Let's keep the issue open to see if it gets traction.

rolfsmeds avatar Dec 15 '25 08:12 rolfsmeds

Maybe some context, why I wrote this issue.

I use the grid in a vertical layout with align-items: stretched. Since the all rows visible also sets a width: 100% I never stumbled upon this align-self, cause it worked out of the box (I assume, that this constellation is the most common use case and therefore there are no tickets).

After I migrated my project to V25, for whatever reason, the width: 100% was not added anymore to my grid with all rows visible and my grid shrinked to a "dashed line". Unfortunately I cannot reproduce this issue in a starter, since there the width: 100% is added as expected.

I could have also created a ticket about "the width: 100% is missing", but I assumed a more general approach more useful - since there might also be other use cases, where the parent is not using stretched. Instead someone could also use something like center or whatever and in this case the width: 100% might not "cover" the align-self.

So if there is a technical reason for the flex-start alignment, than it is what it is - but then the documentation should mention clear enough, how to overcome this problem, so that we don't have a bunch of angry java devs on our door mat, who now have to work with css and browser tools out of the sudden ;)

stefanuebe avatar Dec 15 '25 09:12 stefanuebe

Perhaps we should instead address the missing width:100%, as it seems to be a breaking change, rather than introduce another breaking change. (@jouni is this an unintentional css change in V25?)

rolfsmeds avatar Dec 15 '25 23:12 rolfsmeds

Base styles for [all-rows-visible] are indeed different from Lumo in V25:

https://github.com/vaadin/web-components/blob/5c73dd6cd70fb5e8d7b8db158e38fb54ce1b645e/packages/grid/src/styles/vaadin-grid-base-styles.js#L51-L57

https://github.com/vaadin/web-components/blob/5c73dd6cd70fb5e8d7b8db158e38fb54ce1b645e/packages/vaadin-lumo-styles/src/components/grid.css#L54-L60

web-padawan avatar Dec 16 '25 06:12 web-padawan

Seems to me that width: 100% was added to address this exact issue.

I suppose it hasn't been an issue to anyone, but it feels a bit strange to only set the width for the "all rows visible" case but not for the default case. Would feel more predictable to use it for all grids by default.

jouni avatar Dec 16 '25 09:12 jouni

IMO we can restore it for [all-rows-visible] and then consider changing the default. I can make a PR.

web-padawan avatar Dec 16 '25 09:12 web-padawan