From a7904c8e3ba0ab67f2aa6bcaf5de2d0f34bd8f08 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Fri, 2 Aug 2024 23:12:03 +0200 Subject: [PATCH 1/5] Ensure range selection apply to current view --- panel/models/tabulator.ts | 62 +++++++++--------------- panel/tests/ui/widgets/test_tabulator.py | 37 ++++++++++++++ panel/tests/widgets/test_tables.py | 9 ++++ panel/widgets/tables.py | 3 +- 4 files changed, 72 insertions(+), 39 deletions(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index e996d231fe..af8dda750d 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -312,6 +312,7 @@ export class DataTabulatorView extends HTMLBoxView { _updating_page: boolean = false _updating_sort: boolean = false _selection_updating: boolean = false + _last_selected_row: any = null _initializing: boolean _lastVerticalScrollbarTopPosition: number = 0 _lastHorizontalScrollbarLeftPosition: number = 0 @@ -1233,44 +1234,22 @@ export class DataTabulatorView extends HTMLBoxView { const selected = this.model.source.selected const index: number = row._row.data._index - if (this.model.pagination === "remote") { - const includes = this.model.source.selected.indices.indexOf(index) == -1 - const flush = !(e.ctrlKey || e.metaKey || e.shiftKey) - if (e.shiftKey && selected.indices.length) { - const start = selected.indices[selected.indices.length-1] - if (index>start) { - for (let i = start; i<=index; i++) { - indices.push(i) - } - } else { - for (let i = start; i>=index; i--) { - indices.push(i) - } - } - } else { - indices.push(index) - } - this._selection_updating = true - this.model.trigger_event(new SelectionEvent(indices, includes, flush)) - this._selection_updating = false - return - } - if (e.ctrlKey || e.metaKey) { - indices = [...this.model.source.selected.indices] - } else if (e.shiftKey && selected.indices.length) { - const start = selected.indices[selected.indices.length-1] - if (index>start) { - for (let i = start; iindex; i--) { - indices.push(i) - } + indices = [...selected.indices] + } else if (e.shiftKey && this._last_selected_row) { + const rows = row._row.parent.getDisplayRows() + const start_idx = rows.indexOf(this._last_selected_row) + if (start_idx !== -1) { + const end_idx = rows.indexOf(row._row) + const reverse = start_idx > end_idx + const [start, end] = reverse ? [end_idx+1, start_idx+1] : [start_idx, end_idx] + indices = rows.slice(start, end).map((r) => r.data._index) + if (reverse) { indices = indices.reverse() } } } - if (indices.indexOf(index) < 0) { + const flush = !(e.ctrlKey || e.metaKey || e.shiftKey) + const includes = indices.includes(index) + if (!includes) { indices.push(index) } else { indices.splice(indices.indexOf(index), 1) @@ -1281,11 +1260,18 @@ export class DataTabulatorView extends HTMLBoxView { indices.shift() } } + const remote = this.model.pagination === "remote" const filtered = this._filter_selected(indices) - this.tabulator.deselectRow() - this.tabulator.selectRow(filtered) + if (!remote) { + this.tabulator.deselectRow() + this.tabulator.selectRow(filtered) + } + this._last_selected_row = row._row this._selection_updating = true - selected.indices = filtered + if (!remote) { + selected.indices = filtered + } + this.model.trigger_event(new SelectionEvent(indices, !includes, flush)) this._selection_updating = false } diff --git a/panel/tests/ui/widgets/test_tabulator.py b/panel/tests/ui/widgets/test_tabulator.py index 664a7b8edc..5e67313154 100644 --- a/panel/tests/ui/widgets/test_tabulator.py +++ b/panel/tests/ui/widgets/test_tabulator.py @@ -3411,6 +3411,42 @@ def test_tabulator_remote_pagination_auto_page_size_shrink(page, df_mixed): +@pytest.mark.parametrize('pagination', ['remote', 'local', None]) +def test_range_selection_on_sorted_data_downward(page, pagination): + df = pd.DataFrame({'a': [1, 3, 2, 4, 5, 6, 7, 8, 9], 'b': [6, 5, 6, 7, 7, 7, 7, 7, 7]}) + table = Tabulator(df, disabled=True, pagination=pagination) + + serve_component(page, table) + + page.locator('.tabulator-col-title-holder').nth(2).click() + + page.locator('.tabulator-row').nth(0).click() + + page.keyboard.down('Shift') + + page.locator('.tabulator-row').nth(1).click() + + wait_until(lambda: table.selection == [0, 2], page) + + +@pytest.mark.parametrize('pagination', ['remote', 'local', None]) +def test_range_selection_on_sorted_data_upward(page, pagination): + df = pd.DataFrame({'a': [1, 3, 2, 4, 5, 6, 7, 8, 9], 'b': [6, 5, 6, 7, 7, 7, 7, 7, 7]}) + table = Tabulator(df, disabled=True, pagination=pagination, page_size=3) + + serve_component(page, table) + + page.locator('.tabulator-col-title-holder').nth(2).click() + + page.locator('.tabulator-row').nth(1).click() + + page.keyboard.down('Shift') + + page.locator('.tabulator-row').nth(0).click() + + wait_until(lambda: table.selection == [2, 0], page) + + class Test_RemotePagination: @pytest.fixture(autouse=True) @@ -3430,6 +3466,7 @@ def check_selected(self, page, expected, ui_count=None): ui_count = len(expected) expect(page.locator('.tabulator-selected')).to_have_count(ui_count) + wait_until(lambda: self.widget.selection == expected, page) @contextmanager diff --git a/panel/tests/widgets/test_tables.py b/panel/tests/widgets/test_tables.py index 015a27791a..3f94ed3b24 100644 --- a/panel/tests/widgets/test_tables.py +++ b/panel/tests/widgets/test_tables.py @@ -238,6 +238,15 @@ def test_none_table(document, comm): assert model.source.data == {} +def test_tabulator_selection_resets(): + df = makeMixedDataFrame() + table = Tabulator(df, selection=list(range(len(df)))) + + for i in reversed(range(len(df))): + table.value = df.iloc[:i] + assert table.selection == list(range(i)) + + def test_tabulator_selected_dataframe(): df = makeMixedDataFrame() table = Tabulator(df, selection=[0, 2]) diff --git a/panel/widgets/tables.py b/panel/widgets/tables.py index 480bd17ea0..a7c9c05d61 100644 --- a/panel/widgets/tables.py +++ b/panel/widgets/tables.py @@ -1287,7 +1287,8 @@ def _cleanup(self, root: Model | None = None) -> None: def _process_event(self, event) -> None: if event.event_name == 'selection-change': - self._update_selection(event) + if self.pagination == 'remote': + self._update_selection(event) return event_col = self._renamed_cols.get(event.column, event.column) From 9f1465deba80cdd98966e36490eefc921cfeb9ff Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Fri, 2 Aug 2024 23:14:40 +0200 Subject: [PATCH 2/5] Fix tabs --- panel/models/tabulator.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index af8dda750d..5b87e2c98b 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -786,7 +786,7 @@ export class DataTabulatorView extends HTMLBoxView { _expand_render(cell: any): string { const index = cell._cell.row.data._index const icon = this.model.expanded.indexOf(index) < 0 ? "►" : "▼" - return `${icon}` + return icon } _update_expand(cell: any): void { @@ -1240,11 +1240,11 @@ export class DataTabulatorView extends HTMLBoxView { const rows = row._row.parent.getDisplayRows() const start_idx = rows.indexOf(this._last_selected_row) if (start_idx !== -1) { - const end_idx = rows.indexOf(row._row) - const reverse = start_idx > end_idx - const [start, end] = reverse ? [end_idx+1, start_idx+1] : [start_idx, end_idx] - indices = rows.slice(start, end).map((r) => r.data._index) - if (reverse) { indices = indices.reverse() } + const end_idx = rows.indexOf(row._row) + const reverse = start_idx > end_idx + const [start, end] = reverse ? [end_idx+1, start_idx+1] : [start_idx, end_idx] + indices = rows.slice(start, end).map((r) => r.data._index) + if (reverse) { indices = indices.reverse() } } } const flush = !(e.ctrlKey || e.metaKey || e.shiftKey) From 5fd15e14ee3c1cabbc40c27fe6cfe4244d18fb0c Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Fri, 2 Aug 2024 23:18:11 +0200 Subject: [PATCH 3/5] Fix type --- panel/models/tabulator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index 5b87e2c98b..2764a99c89 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -1243,7 +1243,7 @@ export class DataTabulatorView extends HTMLBoxView { const end_idx = rows.indexOf(row._row) const reverse = start_idx > end_idx const [start, end] = reverse ? [end_idx+1, start_idx+1] : [start_idx, end_idx] - indices = rows.slice(start, end).map((r) => r.data._index) + indices = rows.slice(start, end).map((r: any) => r.data._index) if (reverse) { indices = indices.reverse() } } } From 0ec5333516b0d03883f0e0e30373ff153878485c Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Fri, 2 Aug 2024 23:29:38 +0200 Subject: [PATCH 4/5] Revert test --- panel/tests/widgets/test_tables.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/panel/tests/widgets/test_tables.py b/panel/tests/widgets/test_tables.py index d7fdf19ccc..c874a5b32d 100644 --- a/panel/tests/widgets/test_tables.py +++ b/panel/tests/widgets/test_tables.py @@ -240,15 +240,6 @@ def test_none_table(document, comm): assert model.source.data == {} -def test_tabulator_selection_resets(): - df = makeMixedDataFrame() - table = Tabulator(df, selection=list(range(len(df)))) - - for i in reversed(range(len(df))): - table.value = df.iloc[:i] - assert table.selection == list(range(i)) - - def test_tabulator_selected_dataframe(): df = makeMixedDataFrame() table = Tabulator(df, selection=[0, 2]) From 8771ae9784a7feb0a92962a99c1339f25f476319 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Sat, 3 Aug 2024 10:38:38 +0200 Subject: [PATCH 5/5] Fix toggling when remote --- panel/models/tabulator.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index 2764a99c89..6befdceed4 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -1249,7 +1249,10 @@ export class DataTabulatorView extends HTMLBoxView { } const flush = !(e.ctrlKey || e.metaKey || e.shiftKey) const includes = indices.includes(index) - if (!includes) { + const remote = this.model.pagination === "remote" + + // Toggle the index on or off (if remote we let Python do the toggling) + if (!includes || remote) { indices.push(index) } else { indices.splice(indices.indexOf(index), 1) @@ -1260,7 +1263,6 @@ export class DataTabulatorView extends HTMLBoxView { indices.shift() } } - const remote = this.model.pagination === "remote" const filtered = this._filter_selected(indices) if (!remote) { this.tabulator.deselectRow()