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

TreeGrid hierarchy column wraps children horizontally #2429

Closed
mvysny opened this issue Dec 15, 2021 · 8 comments · Fixed by vaadin/web-components#3525
Closed

TreeGrid hierarchy column wraps children horizontally #2429

mvysny opened this issue Dec 15, 2021 · 8 comments · Fixed by vaadin/web-components#3525
Assignees
Labels

Comments

@mvysny
Copy link
Member

mvysny commented Dec 15, 2021

Description

Hierarchy column lays out its children in a different way than a non-hierarchy column: hierarchy column wraps its children while non-hierarchy column fills its parent. This produces an inconsistency of behavior for components that rely on width-filling the column, for example a Div which ellipsizes its text.

Expected outcome

Hierarchy column should make its child fill-parent, exactly as a regular column does.

Actual outcome

Hierarchy column leaves the child to grow, overflowing the column contents.

Live Demo (optional)

Screenshot from 2021-12-15 12-27-16

Minimal reproducible example

package org.vaadin.example;

import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.component.treegrid.TreeGrid;
import com.vaadin.flow.router.Route;

import java.util.ArrayList;
import java.util.Arrays;

@Route("")
public class MainView extends VerticalLayout {

    public MainView() {
        final TreeGrid<String> grid = new TreeGrid<>();
        grid.setItems(Arrays.asList("1", "2", "3"), it -> new ArrayList<>());
        grid.addComponentHierarchyColumn(it -> {
            final Div div = new Div();
            div.setText("this one isn't fill parent and will overflow happily " + it);
            div.getStyle().set("text-overflow", "ellipsis");
            div.getStyle().set("overflow", "hidden");
            return div;
        }).setHeader("hierarchical").setResizable(true);
        grid.addComponentColumn(it -> {
            final Div div = new Div();
            div.setText("this one fills the column correctly and will thus ellipsize properly " + it);
            div.getStyle().set("text-overflow", "ellipsis");
            div.getStyle().set("overflow", "hidden");
            return div;
        }).setHeader("hierarchical").setResizable(true);
        add(grid);
    }
}

Steps to reproduce

Paste the class above to the skeleton-starter-flow project.

Environment

  • Vaadin 21.0.9
@mvysny
Copy link
Member Author

mvysny commented Dec 15, 2021

A workaround is to use the following CSS (a global CSS, no need to inject it into the TreeGrid or Grid):

vaadin-grid-tree-toggle {
  width: 100%;
}
vaadin-grid-tree-toggle > flow-component-renderer {
  width: calc(100% - 1.5em);
}

@rolfsmeds
Copy link
Contributor

Changed the title to better reflect what's going on here:
The toggle element doesn't stay within the bounds of the cell, but instead overflows it. The content is still visually truncated because it ends up behind behind the following cell.

@web-padawan web-padawan self-assigned this Mar 7, 2022
@web-padawan
Copy link
Member

web-padawan commented Mar 7, 2022

Looks like fixing this in the web component would require us to align vaadin-grid-tree-toggle with vaadin-grid-sorter:

  1. Add part to wrap <slot> element:
<div part="content">
  <slot></slot>
</div>
  1. Add corresponding CSS to content part:
[part='content'] {
  overflow: hidden;
  text-overflow: ellipsis;
}
  1. Set max-width: 100% on the vaadin-grid-tree-toggle:
:host {
  max-width: 100%;
}

The main reason behind adding new stylable part is the fact that we need an extra HTML element to apply styles.
We can't use ::slotted(*) as it won't select a text node placed into a slot; it only targets actual elements.

UPD: the last point can be done separately as a bugfix. Then it would be easier to make this work with Flow.

@rolfsmeds
Copy link
Contributor

If we're going to mess with the DOM, would it make sense to do #1091 at the same time?

@jouni
Copy link
Member

jouni commented Mar 7, 2022

Instead of adding a new element, we can also style the slot element. Not sure if that helps?

@rolfsmeds
Copy link
Contributor

Not really, since the slot is display:contents we specifically can not style the slot per se, right? We can apply styles that are inherited by the slots contents, but that won't help either since the content is a text node.

@web-padawan
Copy link
Member

web-padawan commented Mar 8, 2022

Instead of adding a new element, we can also style the slot element

We actually can set display: block on the slot element. I tried it, seems to work in all browsers 🙈

UPD: here is a PR vaadin/web-components#3525

@jouni
Copy link
Member

jouni commented Mar 8, 2022

We actually can set display: block on the slot element.

Yeah, this is what I meant. I think it’s a completely valid approach, if you can't think of reasons why it wouldn't work. I mean, that could also be considered as “messing with the DOM”?

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

Successfully merging a pull request may close this issue.

4 participants