GridSelectionModel does not behave as expected when adding selection listeners (too early)

[StefanPenndorf]

Currently ComponentEventBus.fireEvent() notifies listeners for concrete event class, only. A change to this behaviour has already been discussed in vaadin/flow#500 and abandoned because of missing use cases.

GridSelectionModel makes use of a basic interface (SelectionEvent) and two concrete event classes (SingleSelectionEvent and MultiSelectionEvent). Users can only register abstract listeners for SelectionEvent directly on the grid. This leads to unexpected behaviour depending on the order of methods called.

Example:

        final Grid<Person> grid = new Grid<>(Person.class);
        final SelectionListener<Grid<Person>, Person> firstListener = event -> {
            System.out.println("first listener (single) selection event");
        };
        final SelectionListener<Grid<Person>, Person> secondListener = event -> {
            System.out.println("second listener (multi) selection event");
        };

        grid.addSelectionListener(firstListener);
        grid.setSelectionMode(Grid.SelectionMode.MULTI);
        grid.addSelectionListener(secondListener);

        // selection event fired here only invokes the second listener.

Per default (relates to Vaadin 23, didn't check Vaadin 24) a SingleSelectionModel ist set. Although the listener is declared as listener for SelectionEvent which could be either SingleSelectionEvent or MultiSelectionEvent the addSelectionListener() method of AbstractGridSingleSelectionModel is called:

    @Override
    public Registration addSelectionListener(SelectionListener<Grid<T>, T> listener) {
        Objects.requireNonNull(listener, "listener cannot be null");
        return ComponentUtil.addListener(getGrid(), SingleSelectionEvent.class, (ComponentEventListener) (event -> listener.selectionChange((SelectionEvent) event)));
    }

that implicitly registers the listener for SingleSelectionEvent only.

On the other hand calling addSelectionListener() after grid.setSelectionMode(Grid.SelectionMode.MULTI) has been called the method on AbstractGridMultiSelectionModel is invoked which registers the selection listener for MultiSelectionEvent instead.

This is unexpected to me.

Bonus: Switching selection mode back to grid.setSelectionMode(Grid.SelectionMode.SINGLE "reactivates" the first listener but renders the second one useless.

Use Case

We've got an overview page with a grid that displays invoice data. The user can switch to mass correction mode and select multiple invoices for mass correction. Because the check if the invoice may be corrected is complex (and takes multiple database queries per invoice) we don't want to always enable mass correction.

I wanted to add a selection listener during Grid creation time but this one is not invoked. Other listeners added after the switch to MultiSelectionModel work fine.

Hack / Workaround

I've added the following method to the grid to perform the "correct" registration before changing selection modes:

    @SuppressWarnings({"unchecked", "rawtypes"})
    public Registration addMultiSelectionListener(final MultiSelectionListener<InvoiceGrid, Invoice> listener) {
        Objects.requireNonNull(listener, "listener cannot be null");
        return ComponentUtil.addListener(
                this,
                MultiSelectionEvent.class,
                (ComponentEventListener) (event -> listener.selectionChange((MultiSelectionEvent) event)));
    }

Possible Solutions

@Deprecate and remove addSelectionListener() on Grid as well as SelectionListener interface

If the decision made in vaadin/flow#500 holds that event type inheritance is not supported or intended, SelectionListener interface contradicts ComponentEventBus usage principles.

The simplest solution would be to remove the method on the grid and the "untyped" SelectionListener interface forcing the users to correctly use the API as intended with explicit event types:

1. add selection listeners on the selection model via ((GridSingleSelectionModel)grid.getSelectionModel()).addSingleSelectionListener(listener)
2. don't confuse users that they could listen to SingleSelectionEvent and MultiSelectionEvent at the same time.

Make ComponentEventBus firing events for subclasses as well

As suggested in vaadin/flow#500 . Selection Models could register listeners for SelectionEvent to be able to catch SingleSelectionEvent and MultiSelectionEvent at the same time.

Provide addMultiSelectionListener() and addSingleSelectionListener() on Grid

Another possibility would be to add those methods to Grid and let the user fiddle out if addSelectionListener() addMultiSelectionListener() or addSingleSelectionListener() is appropriate for her use case. This is not my favourite solution, though because it still leaves the user with odd behaviour if she doesn't read the javadoc or documentation.