Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Tabulator selection consistency when pagination='local' #7304

Merged
merged 12 commits into from
Sep 23, 2024
99 changes: 54 additions & 45 deletions panel/models/tabulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {debounce} from "debounce"
import {comm_settings} from "./comm_manager"
import {transform_cds_to_records} from "./data"
import {HTMLBox, HTMLBoxView} from "./layout"
import {schedule_when} from "./util"

export class TableEditEvent extends ModelEvent {
constructor(readonly column: string, readonly row: number, readonly pre: boolean) {
Expand Down Expand Up @@ -391,7 +392,7 @@ export class DataTabulatorView extends HTMLBoxView {
this.tabulator.download(ftype, this.model.filename)
})

this.on_change(children, () => this.renderChildren())
this.on_change(children, () => this.renderChildren(false))

this.on_change(expanded, () => {
// The first cell is the cell of the frozen _index column.
Expand Down Expand Up @@ -501,25 +502,27 @@ export class DataTabulatorView extends HTMLBoxView {
}
if (rows && (this.tabulator.rowManager.renderer != null)) {
this.tabulator.rowManager.redraw(true)
this.renderChildren()
this.setStyles()
}
this._redrawing = false
this._restore_scroll = true
}

get is_drawing(): boolean {
return this._building || this._redrawing || !this.root.has_finished()
}

override after_layout(): void {
super.after_layout()
const finished = this.root.has_finished()
if (this.tabulator != null && this._initializing && !this._building && finished) {
if (this.tabulator != null && this._initializing && !this.is_drawing) {
this._initializing = false
this.redraw()
this._resize_redraw()
}
}

override after_resize(): void {
super.after_resize()
if (!this._is_scrolling && !this._redrawing) {
if (!this._is_scrolling && !this._initializing && !this.is_drawing) {
this._debounced_redraw()
}
}
Expand Down Expand Up @@ -550,6 +553,7 @@ export class DataTabulatorView extends HTMLBoxView {
}
super.render()
this._initializing = true
this._building = true
const container = div({style: {display: "contents"}})
const el = div({style: {width: "100%", height: "100%", visibility: "hidden"}})
this.container = el
Expand Down Expand Up @@ -602,7 +606,9 @@ export class DataTabulatorView extends HTMLBoxView {
}, 50, false))

// Sync state with model
this.tabulator.on("rowSelectionChanged", (data: any, rows: any, selected: any, deselected: any) => this.rowSelectionChanged(data, rows, selected, deselected))
this.tabulator.on("rowSelectionChanged", (data: any, rows: any, selected: any, deselected: any) => {
this.rowSelectionChanged(data, rows, selected, deselected)
})
this.tabulator.on("rowClick", (e: any, row: any) => this.rowClicked(e, row))
this.tabulator.on("cellEdited", (cell: any) => this.cellEdited(cell))
this.tabulator.on("dataFiltering", (filters: any) => {
Expand Down Expand Up @@ -694,19 +700,13 @@ export class DataTabulatorView extends HTMLBoxView {
this.tabulator.setPage(this.model.page)
}
this._building = false
if (this._initializing && this.root.has_finished()) {
schedule_when(() => {
const initializing = this._initializing
this._initializing = false
this.redraw()
} else if (!this.root.has_finished()) {
const finalize = () => {
if (this.root.has_finished()) {
this._initializing = false
} else {
setTimeout(finalize, 10)
}
if (initializing) {
this._resize_redraw()
}
setTimeout(finalize, 10)
}
}, () => this.root.has_finished())
}

requestPage(page: number, sorters: any[]): Promise<void> {
Expand Down Expand Up @@ -772,21 +772,13 @@ export class DataTabulatorView extends HTMLBoxView {
frozenRows: (row: any) => {
return (this.model.frozen_rows.length > 0) ? this.model.frozen_rows.includes(row._row.data._index) : false
},
rowFormatter: (row: any) => this._render_row(row),
}
if (this.model.pagination === "remote") {
configuration.ajaxURL = "http://panel.pyviz.org"
configuration.sortMode = "remote"
}
const cds: any = this.model.source
let data: any[]
if (cds === null || (cds.columns().length === 0)) {
data = []
} else {
data = transform_cds_to_records(cds, true)
}
if (configuration.dataTree) {
data = group_data(data, this.model.columns, this.model.indexes, this.model.aggregators)
}
const data = this.getData()
return {
...configuration,
data,
Expand All @@ -811,19 +803,27 @@ export class DataTabulatorView extends HTMLBoxView {
return children
}

renderChildren(): void {
get row_index(): Map<number, any> {
const rows = this.tabulator.getRows()
const lookup = new Map()
for (const row of rows) {
const index = row._row?.data._index
if (index != null) {
lookup.set(index, row)
}
}
return lookup
}

renderChildren(all: boolean = true): void {
new Promise(async (resolve: any) => {
const new_children = await this.build_child_views()
let new_children = await this.build_child_views()
if (all) {
new_children = this.child_views
}
resolve(new_children)
}).then((new_children) => {
const rows = this.tabulator.getRows()
const lookup = new Map()
for (const row of rows) {
const index = row._row?.data._index
if (index != null) {
lookup.set(index, row)
}
}
const lookup = this.row_index
for (const index of this.model.expanded) {
const model = this.get_child(index)
const row = lookup.get(index)
Expand Down Expand Up @@ -859,14 +859,19 @@ export class DataTabulatorView extends HTMLBoxView {
let viewEl
if (prev_child != null && prev_child.className == "row-content") {
viewEl = prev_child
if (viewEl.children.length && viewEl.children[0] === view.el) {
return
}
} else {
viewEl = div({class: "row-content", style: {background_color: bg, margin_left: neg_margin, max_width: "100%", overflow_x: "hidden"}})
rowEl.appendChild(viewEl)
}
viewEl.appendChild(view.el)
rowEl.appendChild(viewEl)
if (!view.has_finished() && render) {
view.render()
view.after_render()
if (render) {
schedule_when(() => {
view.render()
view.after_render()
}, () => this.root.has_finished())
}
if (resize) {
this._update_children()
Expand Down Expand Up @@ -914,7 +919,13 @@ export class DataTabulatorView extends HTMLBoxView {
}

getData(): any[] {
let data = transform_cds_to_records(this.model.source, true)
const cds = this.model.source
let data: any[]
if (cds === null || (cds.columns().length === 0)) {
data = []
} else {
data = transform_cds_to_records(cds, true)
}
if (this.model.configuration.dataTree) {
data = group_data(data, this.model.columns, this.model.indexes, this.model.aggregators)
}
Expand Down Expand Up @@ -1261,15 +1272,13 @@ export class DataTabulatorView extends HTMLBoxView {
setPage(): void {
this.tabulator.setPage(Math.min(this.model.max_page, this.model.page))
if (this.model.pagination === "local") {
this.renderChildren()
this.setStyles()
}
}

setPageSize(): void {
this.tabulator.setPageSize(this.model.page_size)
if (this.model.pagination === "local") {
this.renderChildren()
this.setStyles()
}
}
Expand Down
11 changes: 11 additions & 0 deletions panel/models/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,14 @@ export function find_attributes(text: string, obj: string, ignored: string[]) {

return uniq(matches)
}

export function schedule_when(func: () => void, predicate: () => boolean, timeout: number = 10): void {
const scheduled = () => {
if (predicate()) {
func()
} else {
setTimeout(scheduled, timeout)
}
}
scheduled()
}
16 changes: 15 additions & 1 deletion panel/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ def pytest_addoption(parser):
for marker, info in optional_markers.items():
parser.addoption(f"--{marker}", action="store_true",
default=False, help=info['help'])

parser.addoption('--repeat', action='store',
help='Number of times to repeat each test')

def pytest_configure(config):
for marker, info in optional_markers.items():
Expand All @@ -155,6 +156,19 @@ def pytest_configure(config):

config.addinivalue_line("markers", "internet: mark test as requiring an internet connection")

def pytest_generate_tests(metafunc):
if metafunc.config.option.repeat is not None:
count = int(metafunc.config.option.repeat)

# We're going to duplicate these tests by parametrizing them,
# which requires that each test has a fixture to accept the parameter.
# We can add a new fixture like so:
metafunc.fixturenames.append('tmp_ct')

# Now we parametrize. This is what happens when we do e.g.,
# @pytest.mark.parametrize('tmp_ct', range(count))
# def test_foo(): pass
metafunc.parametrize('tmp_ct', range(count))

def pytest_collection_modifyitems(config, items):
skipped, selected = [], []
Expand Down
Loading
Loading