bug(data frame): Subset selected cells from the filtered rows (#1412)

CHANGELOG.md

@@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * Removed temporary state where a data frame renderer would try to subset to selected rows that did not exist. (#1351, #1377)
 
+* Fix an issue in the data frame output which caused the table to freeze when filters removed previously selected cells. (#1412)
+
 ### Other changes
 
 * `Session` is now an abstract base class, and `AppSession` is a concrete subclass of it. Also, `ExpressMockSession` has been renamed `ExpressStubSession` and is a concrete subclass of `Session`. (#1331)

js/data-frame/index.tsx

@@ -328,7 +328,15 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
       const rowsById = table.getSortedRowModel().rowsById;
       shinyValue = {
         type: "row",
-        rows: rowSelectionKeys.map((key) => rowsById[key].index).sort(),
+        rows: rowSelectionKeys
+          .map((key) => {
+            if (!(key in rowsById)) {
+              return null;
+            }
+            return rowsById[key].index;
+          })
+          .filter((x): x is number => x !== null)
+          .sort(),
       };
     } else {
       console.error("Unhandled row selection mode:", rowSelectionModes);
@@ -370,7 +378,15 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
     if (rowSelectionModes.row !== SelectionModes._rowEnum.NONE) {
       const rowSelectionKeys = rowSelection.keys().toList();
       const rowsById = table.getSortedRowModel().rowsById;
-      shinyValue = rowSelectionKeys.map((key) => rowsById[key].index).sort();
+      shinyValue = rowSelectionKeys
+        .map((key) => {
+          if (!(key in rowsById)) {
+            return null;
+          }
+          return rowsById[key].index;
+        })
+        .filter((x): x is number => x !== null)
+        .sort();
     }
     Shiny.setInputValue!(`${id}_selected_rows`, shinyValue);
   }, [id, rowSelection, rowSelectionModes, table]);

tests/playwright/shiny/bugs/1390-df-selected-row-filtered/app.py

@@ -0,0 +1,29 @@
+import pandas as pd
+
+from shiny import req
+from shiny.express import render, ui
+
+df = pd.DataFrame(data={"a": [str(i) for i in range(10)]})
+
+
+with ui.layout_column_wrap(width=1 / 2):
+    with ui.card():
+        ui.card_header("Original data")
+
+        @render.data_frame
+        def my_df():
+            return render.DataGrid(data=df, filters=True, selection_mode="rows")
+
+    with ui.card():
+        ui.card_header("Selected data")
+
+        @render.data_frame
+        def selected_df():
+            return my_df.data_view(selected=True)
+
+    with ui.card():
+        ui.card_header("Selected rows")
+
+        @render.code
+        def selected_rows():
+            return str(req(my_df.cell_selection() or {}).get("rows", ()))

tests/playwright/shiny/bugs/1390-df-selected-row-filtered/test_1390_selected_row_filtered.py

@@ -0,0 +1,87 @@
+from __future__ import annotations
+
+import platform
+
+from conftest import ShinyAppProc
+from controls import OutputCode, OutputDataFrame
+from playwright.sync_api import Page, expect
+
+
+def test_row_selection(page: Page, local_app: ShinyAppProc) -> None:
+    page.goto(local_app.url)
+
+    my_df = OutputDataFrame(page, "my_df")
+    selected_df = OutputDataFrame(page, "selected_df")
+    selected_rows = OutputCode(page, "selected_rows")
+
+    # TODO-karan-test: We should add a locator for selected rows
+    # TODO-karan-test; We should add a expected selected row count method?
+    def expect_selected_count(x: OutputDataFrame, n: int) -> None:
+        expect(x.loc_body.locator("tr[aria-selected=true]")).to_have_count(n)
+
+    my_df.expect_n_row(10)
+    expect_selected_count(my_df, 0)
+    selected_df.expect_n_row(0)
+    selected_rows.expect_value("()")
+
+    # Select row
+    my_df.select_rows([5, 7])
+    my_df.expect_n_row(10)
+    expect_selected_count(my_df, 2)
+    selected_df.expect_n_row(2)
+    selected_rows.expect_value("(5, 7)")
+
+    # # Filter to different row
+    # Currently this causes an error
+    my_df.set_column_filter(0, text="5")
+    my_df.expect_n_row(1)
+    expect_selected_count(my_df, 1)
+    selected_df.expect_n_row(1)
+    selected_rows.expect_value("(5,)")
+    # # Remove the filter
+    my_df.set_column_filter(0, text="")
+    # Confirm the original data frame returns
+    my_df.expect_n_row(10)
+    # Confirm the previous row is still selected as no new selection has been made
+    expect_selected_count(my_df, 2)
+    selected_df.expect_n_row(2)
+    selected_rows.expect_value("(5, 7)")
+
+    # Filter to non selected row
+    # Currently this causes an error
+    my_df.set_column_filter(0, text="8")
+    my_df.expect_n_row(1)
+    expect_selected_count(my_df, 0)
+    selected_df.expect_n_row(0)
+    selected_rows.expect_value("()")
+
+    # Remove the filter
+    # Confirm the original data frame returns
+    my_df.set_column_filter(0, text="")
+    my_df.expect_n_row(10)
+    expect_selected_count(my_df, 2)
+    selected_df.expect_n_row(2)
+    selected_rows.expect_value("(5, 7)")
+
+    # Update selection with new selection while under a filter
+    my_df.set_column_filter(0, text="8")
+    my_df.expect_n_row(1)
+    expect_selected_count(my_df, 0)
+    selected_df.expect_n_row(0)
+    selected_rows.expect_value("()")
+    my_df.select_rows([0])
+    my_df.set_column_filter(0, text="")
+    my_df.expect_n_row(10)
+    expect_selected_count(my_df, 1)
+    selected_df.expect_n_row(1)
+    selected_rows.expect_value("(8,)")
+
+    # Remove selection
+    modifier = "Meta" if platform.system() == "Darwin" else "Control"
+    for row in (8,):
+        my_df.cell_locator(row=row, col=0).click(
+            modifiers=[modifier]
+        )  # TODO-karan-test: Support unselect row method
+    expect_selected_count(my_df, 0)
+    selected_df.expect_n_row(0)
+    selected_rows.expect_value("()")