plotly / dash-ag-grid

Dash AG Grid is a high-performance and highly customizable component that wraps AG Grid, designed for creating rich datagrids.
https://dash.plotly.com/dash-ag-grid
MIT License
175 stars 25 forks source link

[BUG] Fatal bug in v31.0.0 release related to selectedRows #267

Closed yxc8 closed 8 months ago

yxc8 commented 8 months ago

Here is a minimal example. One cannot select row on the front end because for some reason every click trigger the callback twice. The selectedRows value get magically changed between when first callback ends and when second callback is triggered. This only exists for v31.0.0, the previous release version v2.4.0 works just fine.

import dash_ag_grid as dag
import pandas as pd
from dash import dcc, html, Input, Output, no_update, State
import dash
# Sample data
data = {'Name': ['John', 'Jane', 'Bob', 'Alice'],
        'Age': [25, 30, 35, 28],
        'City': ['New York', 'San Francisco', 'Los Angeles', 'Chicago']}

df = pd.DataFrame(data)

app = dash.Dash(__name__)
app.layout = dag.AgGrid(
    id='my_table',
    columnDefs=[
        {'headerName': 'Name', 'field': 'Name', "checkboxSelection": True},
        {'headerName': 'Age', 'field': 'Age'},
        {'headerName': 'City', 'field': 'City'}
    ],
    rowData=df.to_dict('records'),
    defaultColDef={"resizable": True, "sortable": True, "filter": True, "wrapText": True,
                   "autoHeight": True, "cellStyle": {'textAlign': 'left', 'whiteSpace': 'pre-wrap'}},
    columnSize="sizeToFit",
    className="ag-theme-alpine selection",
    dashGridOptions={
        "rowSelection": "single",
        "suppressNoRowsOverlay": "true",
    },
    style={"height": "400px"}
)

@dash.callback(
    Output(component_id='my_table', component_property='rowData'),
    Input(component_id="my_table", component_property="selectedRows"),
    State(component_id='my_table', component_property='rowData'),
    config_prevent_initial_callbacks=True
)
def update_qa_area(selected_row, data):
    trigger_id = dash.callback_context.triggered[0]['prop_id'].split('.')[0]
    trigger_property = dash.callback_context.triggered[0]['prop_id'].split('.')[1] if trigger_id else ""
    print(f"Callback triggered: {trigger_id}, {trigger_property}, {selected_row}")
    print(f"Output: {selected_row}")
    return data

if __name__ == '__main__':
    app.run_server(debug=True)
AnnMarieW commented 8 months ago

Hi @yxc8

I ran your example in both 2.40 and 31.0.0 and I can verify your results. However, I think V31.0,0 is running as expected. When you return all the rowData it is recreating the grid which clears the selectedRows , triggering another callback.

If you return dash.no_update instead of data, then the selected row remains set. I know this is a minimal example, but is there a reason you are returning data?

yxc8 commented 8 months ago

@AnnMarieW Thanks for the response! I do have a reason to return data because this is just a minimal example to reproduce what I have.

In reality, I have a big callback that does almost everything, so there are cases when selected rows needs updating and cases when data needs updating based on different trigger IDs. Even though I can theoretically separate the big callback into multiple, I would run into problems because the order those callbacks are being triggered might not be the same every time.

I think it is very un-intuitive from a developer perspective to have this change in v31.0.0, because a developer would think it is reasonable to assume the property values remain the same between callbacks. How is a developer supposed to know what is being changed in the background and causing another callback to be triggered? Is there a strong reason for this change and is it easy to just keep how things work in v2.4.0?

BSd3v commented 8 months ago

Hello @yxc8,

The if the data is changing, there is no way for the grid to maintain the selection, however, we made a way for the grid to know when something changes and still maintain the selection, you need to provide a getRowId, here is your example using rowData:

import dash_ag_grid as dag
import pandas as pd
from dash import dcc, html, Input, Output, no_update, State
import dash
# Sample data
data = {'Name': ['John', 'Jane', 'Bob', 'Alice'],
        'Age': [25, 30, 35, 28],
        'City': ['New York', 'San Francisco', 'Los Angeles', 'Chicago']}

df = pd.DataFrame(data)

app = dash.Dash(__name__)
app.layout = dag.AgGrid(
    id='my_table',
    columnDefs=[
        {'headerName': 'Name', 'field': 'Name', "checkboxSelection": True},
        {'headerName': 'Age', 'field': 'Age'},
        {'headerName': 'City', 'field': 'City'}
    ],
    rowData=df.to_dict('records'),
    defaultColDef={"resizable": True, "sortable": True, "filter": True, "wrapText": True,
                   "autoHeight": True, "cellStyle": {'textAlign': 'left', 'whiteSpace': 'pre-wrap'}},
    columnSize="sizeToFit",
    className="ag-theme-alpine selection",
    dashGridOptions={
        "rowSelection": "single",
        "suppressNoRowsOverlay": "true",
    },
    getRowId="params.data.Name",
    style={"height": "400px"}
)

@dash.callback(
    Output(component_id='my_table', component_property='rowData'),
    Input(component_id="my_table", component_property="selectedRows"),
    State(component_id='my_table', component_property='rowData'),
    config_prevent_initial_callbacks=True
)
def update_qa_area(selected_row, data):
    trigger_id = dash.callback_context.triggered[0]['prop_id'].split('.')[0]
    trigger_property = dash.callback_context.triggered[0]['prop_id'].split('.')[1] if trigger_id else ""
    print(f"Callback triggered: {trigger_id}, {trigger_property}, {selected_row}")
    print(f"Output: {selected_row}")
    return data

if __name__ == '__main__':
    app.run_server(debug=True, port='1234')

With that being said, I normally recommend using rowTransaction instead of updating all the rowData, data transfer is smaller, and it takes less on the browser side to implement.

We actually have a test that performs the same actions.

https://github.com/plotly/dash-ag-grid/blob/main/tests/test_selection_persistence.py

This is an issue with the underlying grid and not our implementation, the grid decides that a row is selected based upon an exact match of the data, how this data is being altered I cannot understand. But please assign a getRowId and you should be fine.

yxc8 commented 8 months ago

Hi @BSd3v , that worked, thank you so much for the suggestion!