1

I'm using a Grid in Vaadin 14. The grid is in multi-selection mode. The selection handler takes a couple of seconds to complete and I'm calling setItems(...) at the end to update the items in the grid.

When the user selects another row while the previous selection handler is still running, I get an "Unknown key" error similar to the one described in https://github.com/vaadin/vaadin-grid-flow/issues/322, even though the new set of items still contains the selected item (another object instance but same according to equals()). This seems to be because the keys in the KeyMapper have already been changed due to setItems(), so the key coming from the client is not present anymore.

Is there a way to work around this, for example by disabling selection while the previous request is in progress?

UPDATE

To work around this Vaadin bug, I'm also calling setPageSize() with the exact number of items as argument. But it seems the same problem occurs even if I don't call setPageSize(), so it's probably due to setItems().

herman
  • 11,740
  • 5
  • 47
  • 58
  • The problem probably comes from you calling `setItems(..)`. Could also be from the pageSize thing, idk. Personally I would reconsider if re-setting the items is the correct thing to do in a selectionListener. You could theoretically make your workaround by spawning a background thread first off in the selectionListener and disable selection by disabling the whole grid using [ui.access(..)](https://vaadin.com/docs/v10/flow/advanced/tutorial-push-access.html). This will then be performed without having to wait for the whole listener to finish. – kscherrer May 26 '20 at 13:53
  • @kscherrer when selecting an item, it must be updated in DB and changes reflected in the grid, and for some items, when deselecting them, they must be removed from DB and the grid. This is the desired functionality. So after DB operations I refresh the grid. Is there a more suitable way to implement this behaviour? Thanks for the suggestion using a background thread, but I was hoping there was a cleaner way. I'm pretty sure now it's from the pageSize, the reset causes the keys to be flushed. – herman May 26 '20 at 14:02
  • @kscherrer OK it seems keys are flushed even when not calling `setPageSize`. So it's due to `setItems(...)` – herman May 26 '20 at 14:13
  • You could wait doing the DB stuff until the user clicks a button, for example "Save selected items". It also sounds a bit like you could do your own column for your desired functionality. Add a column with a Checkbox, and on that you have a valueChangeListener that saves things into DB. – kscherrer May 26 '20 at 14:17
  • Are you using the selection state to define a boolean field of the item? – kscherrer May 26 '20 at 14:38
  • @kscherrer it's more complex: I'm using the selection state to control the presence of this item in another map (which is saved in DB). I've been thinking about making this a dedicated column with a rendered checkbox component, but I'm wondering if this wouldn't result in similar issues when calling `setItems(...)` in the value change handler of the checkbox. – herman May 26 '20 at 15:01
  • yes it will lead to the same issues in that case, the checkbox column idea was for if you simply used it for a boolean value of your item. But.. I don't actually see why there is a need to call setItems(..) in the first place? Its seems like you still want to show ALL the items as were shown before, even those that are not present in that other map. – kscherrer May 26 '20 at 15:11
  • @kscherrer well the grid actually contains some fixed and some additional items. The fixed ones need to be selected iff they are present in the map. The additional ones have to be removed from the grid when deselected (they are added using a popup). – herman May 26 '20 at 15:29

2 Answers2

0

Do not change the grids items inside a SelectionListener. You can still do all the things you wanted, but setting the items anew is not actually needed. In fact it will only create problems as you are experiencing now.

While working at this answer, I realized you will need to do your own Checkbox Column in order to be able to do actions for the one item that was just "selected", instead of removing all then add all selected ones (because much better performance). Here is how that could look.

// in my code samples, a `Foo` item can have many `Bar` items. The grid is of type Bar.

Grid.Column customSelectionColumn = grid.addComponentColumn(item -> {
    Checkbox isSelected = new Checkbox();
    isSelected.setValue(someParentFoo.getBars().contains(item));
    isSelected.addValueChangeListener(event -> {
        boolean newSelectedValue = event.getValue();
        if(newSelectedValue){
            someParentFoo.getBars().add(item)
        } else {
            someParentFoo.getBars().remove(item);
        } 
        fooRepository.save(someParentFoo);
    });
});
// make a Checkbox that selects all in the header
Checkbox toggleSelectAll = new Checkbox();
toggleSelectAll.addValueChangeListener(event -> {
    if(event.getValue()){
        someParentFoo.getBars().addAll(allGridItems);
    } else {
        someParentFoo.getBars().removeAll(allGridItems);
    }
    fooRepository.save(someParentFoo);
    grid.getDataProvider().refreshAll(); // updates custom checkbox value of each item
});
gridHeaderRow.getCell(customSelectionColumn).setComponent(toggleSelectAll);
kscherrer
  • 5,486
  • 2
  • 19
  • 59
  • Unless I'm missing something, this does not allow me to remove certain rows on deselection (as I mentioned in comment thread on the question) right? I may call `setItems` anyway for that case but put a modal delete-confirmation popup in there to avoid the user deselecting something and then selecting something else before the previous event handler was finished. – herman May 26 '20 at 16:09
  • yes. it will *not remove the rows from the grid, but it will remove the relation between foo and the deselected bar* and save that new state in DB. – kscherrer May 26 '20 at 16:13
  • No, **don't** call setItems! – kscherrer May 26 '20 at 16:14
  • I removed the code that was misusing the row selection feature of Vaadins Grid. I think you should re-evaluate step-by-step what the user experience will be like. On top of that, the selectionListener has no API for getting the item whose selection was just toggled, making all this 3 times as complicated, because the user can only select/deselect 1 item at a time. – kscherrer May 26 '20 at 16:26
  • I already have code for figuring out the new selected and deselected items, it's only a couple of lines. But the requirement is really to remove certain lines from the grid on deselection. These items only exist in the collection I'm removing them from. The other items exist in one and selection means that a corresponding item exists in another. For the latter there is no problem but the former need to be removed from the grid. If the user goes to another page and comes back they will have disappeared (because data was removed) so it would be weird if they stayed in the grid at first. – herman May 26 '20 at 22:22
  • Btw I've tried simply recreating the whole grid instead of calling `setItems`, but then the user can still select additional rows during the first selection handler execution and these selections will be ignored. It's a little bit better than an error but still feels weird and easy to make mistakes. – herman May 26 '20 at 22:31
  • I'll work around it by saving+refreshing only after clicking a separate "Save" button. But it won't solve this completely: user will still be able to click in the grid while save is in progress. – herman May 27 '20 at 07:51
  • @herman I just remembered that you can avoid `setItems` by removing/adding items from the original item-collection that you used for your ListDataProvider, then refreshing the grid. Not sure if this helps with the unknown key issue though – kscherrer May 27 '20 at 11:42
0

I solved this problem. Vaadin use data as key in HashMap. You need calc hashCode use immutable data fields. For example

public class TestData {

    private int id;
    private String name;

    public TestData(int id) {
        this.id = id;
    }

    @Override
    public int hashCode() {
        return Objects.hash(id);
    }

    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}
Vasily Menshev
  • 369
  • 3
  • 4