aggrid rowSelected is confusing

[davetapley]

Here's a gotcha:

from nicegui import events, ui

grid = ui.aggrid({
    'defaultColDef': {'flex': 1},
    'columnDefs': [
        {'headerName': 'Name', 'field': 'name'},
        {'headerName': 'Age', 'field': 'age'},
        {'headerName': 'Parent', 'field': 'parent', 'hide': True},
    ],
    'rowData': [
        {'name': 'Alice', 'age': 18, 'parent': 'David'},
        {'name': 'Bob', 'age': 21, 'parent': 'Eve'},
        {'name': 'Carol', 'age': 42, 'parent': 'Frank'},
    ],
    'rowSelection': 'multiple',
}).classes('max-h-40')


label = ui.label('click a row')


def row_selected(event: events.GenericEventArguments):
    label.text = event.args['data']['name']


grid.on('rowSelected', row_selected)

ui.run()

On first click this appears to work correctly, but on subsequent clicks it intermittently lags behind by one click.

After digging around I discovered rowSelected is also fired for the row being 'unselected',
and you have to check which is which in the handler, e.g.:

def row_selected(event: events.GenericEventArguments):
    if not event.args['selected']:
        return

    label.text = event.args['data']['name']

---

I assume rowSelected is coming from aggrid itself, but wow is that confusing.

It would be a breaking change but I would prefer if rowSelected was only for the row actually being selected, and we added e.g. rowUnselected and possibly rowSelectionChanged to preserve existing behavior.

[evnchn]

I am sorry for your pain. It is most definitely not coming from NiceGUI since:

1. All AG Grid events are passed through to NiceGUI via the AG Grid global listener. https://nicegui.io/documentation/aggrid#respond_to_an_ag_grid_event
2. It is "camelCase"
3. Code search comes up dry https://github.com/search?q=repo%3Azauberzeug%2Fnicegui%20rowSelected%20&type=code
4. It's in the AG Grid docs twice, https://www.ag-grid.com/javascript-data-grid/row-events/#reference-rowNodeEvents-rowSelected and https://www.ag-grid.com/javascript-data-grid/grid-events/#reference-selection-rowSelected, both times mentioning clearly "Row was selected or unselected." (note that NiceGUI does the former, since it listens for the entire grid)

Given that it is external library, I think it would not be great, if we "fix" the rowSelected and only that. That would just make the behaviour of NiceGUI + AG Grid even more unpredictable, and worsen things even more.

I think we can add a link to https://www.ag-grid.com/javascript-data-grid/grid-events/#reference-selection-rowSelected for https://nicegui.io/documentation/aggrid#respond_to_an_ag_grid_event. It is the best we can do at the moment.

[falkoschindler]

I agree. Deviating from external libraries tends to be confusing in the long run. By providing a shallow layer around existing libraries we leverage their features and documentation without adding too much complexity to NiceGUI itself.
I mean, we could add additional events on_row_selection_added and on_row_selection_removed. But this would require a non-trivial coordination of when to register which events from the client for re-routing them on the server. I'd rather not do that.
Adding some documentation, like a new demo about row selection, might be useful though.