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

ComponentRenderer and Grid selection Bug #4313

Closed
bisam-rd opened this issue Jun 20, 2018 · 4 comments
Closed

ComponentRenderer and Grid selection Bug #4313

bisam-rd opened this issue Jun 20, 2018 · 4 comments
Assignees
Labels
BFP Bugfix priority, also known as Warranty wontfix
Milestone

Comments

@bisam-rd
Copy link

Description of the bug

We faced an issue to select a row when we use a ComponentRenderer.

In the following example, it's quite tricky to select a row via the second column.

Grid<Person> grid = new Grid<>();
grid.setDataProvider(new ListDataProvider<Person>(persons));

grid.addColumn(person -> person.name)
     .setHeader("col 1 (works)");

grid.addColumn(new ComponentRenderer<>((person)-> new Label(person.name)))
     .setHeader("col 2 (bug)");

We suspect that the ComponentRenderer 'consume' the click event. Using the dev console of chrome, we did not observe any event sent to the backend.

Bug observed in v10.0.0-rc5

@bisam-rd bisam-rd changed the title ComponentRenderer and Grid selection ComponentRenderer and Grid selection Bug Jun 20, 2018
@denis-anisimov denis-anisimov added this to the 1.0 Maintenance milestone Jun 21, 2018
@pleku pleku added the BFP Bugfix priority, also known as Warranty label Jul 4, 2018
@denis-anisimov
Copy link
Contributor

It might be that this bug is already fixed in release.

There is a listener indeed for click events in the renderer.
It has been added in rc5 and removed in release.
In the master branch it's again there but now it should work correctly.
Anyway we reverted the fix which added the click listener in the release and may be this fixed this issue.

@mmerruko
Copy link

mmerruko commented Jul 5, 2018

I took a look at this issue and this is coming from the vaadin-grid web component specifically you can take a look at the method _isFocusable(target) https://github.com/vaadin/vaadin-grid/blob/master/src/vaadin-grid-cell-click-mixin.html.

This will dispatch a cell-activate event on click if the clicked element is not focusable according to its criteria, and label is focusable according to the method. This is not related to the ComponentRenderer as such, you get the same result if you use a TemplateRenderer and render a full-width label. You only get selection changes when you click the edge of the column where the padding space is.

As a solution if you have more complex components that have an impact on the selection you could select the row programmatically.

@heruan
Copy link
Member

heruan commented Jul 5, 2018

@bisam-rd Is there a specific reason for which you are using Label? Note that Label will produce a <label> element for form fields; maybe Text or Span could be better choices.

@gilberto-torrezan gilberto-torrezan self-assigned this Jul 10, 2018
@gilberto-torrezan
Copy link
Contributor

The problem in the provided example is caused exactly by the usage of the Label component.

In V10, the Label html component (from the com.vaadin.flow.component.html package) creates a <label> element in the DOM, which makes it semantically different from the Vaadin 8's Label (which creates a <div>).

The problem with <label> is that it is considered an interactive content element, which makes it behave just like buttons, anchors and inputs - basically they can be focused and receive user input, and that input is ignored by the grid webcomponent when considering the row selection.

For example, if you use a <button> instead of a <label>, the same thing happens (the row is not selected when the button is clicked):

// clicking on the button doesn't trigger the selection
grid.addColumn(new ComponentRenderer<>(
                (person) -> new NativeButton(person.getName())))
                .setHeader("col 2 (bug)");

Also, this is not a bug in the ComponentRenderer - the same behavior can be observed with pure TemplateRenderers as well:

// clicking on the label also doesn't trigger the selection
grid.addColumn(
                TemplateRenderer.<Person> of("<label>[[item.name]]</label>")
                        .withProperty("name", Person::getName))
                .setHeader("col 2 (bug)");

To make interactive content elements to trigger the selection of the row when they are clicked, an explicit click handler needs to be added on the server side:

// this code is needed for any interactive element, such as labels, buttons, and anchors,
// to trigger row selections
grid.addColumn(new ComponentRenderer<>((person) -> {
            Label label = new Label(person.getName());
            label.getElement().addEventListener("click", e -> {
                if (grid.getSelectedItems().contains(person)) {
                    grid.deselect(person);
                } else {
                    grid.select(person);
                }
            });
            return label;
        })).setHeader("col 2");

... but my suggestion is to avoid using the <label> element, since it is not meant to be used for loose text in the page. You can use a <span> or <div> instead, and all the selection mechanism will work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFP Bugfix priority, also known as Warranty wontfix
Projects
None yet
Development

No branches or pull requests

6 participants