From b1fd6df41c188b4b23780b14135bc7a35c8b2504 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Sat, 14 Sep 2024 12:25:50 +0200 Subject: [PATCH 1/4] Various changes to improve Tabulator robustness --- panel/models/tabulator.ts | 41 +++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index 97485cf6f5..dcc3bbba7b 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -359,6 +359,8 @@ export class DataTabulatorView extends HTMLBoxView { _debounced_redraw: any = null _restore_scroll: boolean | "horizontal" | "vertical" = false _updating_scroll: boolean = false + _scroll_timeout: number | null = null + _is_scrolling: boolean = false override connect_signals(): void { super.connect_signals() @@ -514,7 +516,9 @@ export class DataTabulatorView extends HTMLBoxView { override after_resize(): void { super.after_resize() - this._debounced_redraw() + if (!this._is_scrolling) { + this._debounced_redraw() + } } _resize_redraw(): void { @@ -591,8 +595,12 @@ export class DataTabulatorView extends HTMLBoxView { return `${cell.getColumn().getField()}: ${cell.getValue()}` }) this.tabulator.on("scrollVertical", debounce(() => { + clearTimeout(this._scroll_timeout) this.record_scroll() this.setStyles() + this._scroll_timeout = setTimeout(() => { + this._is_scrolling = true + }, 100) }, 50, false)) this.tabulator.on("scrollHorizontal", debounce(() => { this.record_scroll() @@ -740,7 +748,6 @@ export class DataTabulatorView extends HTMLBoxView { paginationSize: this.model.page_size || 20, paginationInitialPage: 1, groupBy: this.groupBy, - rowFormatter: (row: any) => this._render_row(row), frozenRows: (row: any) => { return (this.model.frozen_rows.length > 0) ? this.model.frozen_rows.includes(row._row.data._index) : false }, @@ -765,10 +772,17 @@ export class DataTabulatorView extends HTMLBoxView { } } + get_child(idx: number): LayoutDOM | null { + if (this.model.children instanceof Map) { + return this.model.children.get(idx) || null + } + return null + } + override get child_models(): LayoutDOM[] { const children: LayoutDOM[] = [] for (const idx of this.model.expanded) { - const child = this.model.children.get(idx) + const child = this.get_child(idx) if (child != null) { children.push(child) } @@ -790,11 +804,8 @@ export class DataTabulatorView extends HTMLBoxView { } } for (const index of this.model.expanded) { - if (!this.model.children.has(index)) { - continue - } + const model = this.get_child(index) const row = lookup.get(index) - const model = this.model.children.get(index) const view = model == null ? null : this._child_views.get(model) if (view != null) { const render = (new_children as UIElementView[]).includes(view) @@ -811,10 +822,10 @@ export class DataTabulatorView extends HTMLBoxView { _render_row(row: any, resize: boolean = true, render: boolean = true): void { const index = row._row?.data._index - if (!this.model.expanded.includes(index) || this.model.children.get(index) == null) { + if (!this.model.expanded.includes(index)) { return } - const model = this.model.children.get(index) + const model = this.get_child(index) const view = model == null ? null : this._child_views.get(model) if (view == null) { return @@ -823,7 +834,13 @@ export class DataTabulatorView extends HTMLBoxView { const style = getComputedStyle(this.tabulator.element.children[1].children[0]) const bg = style.backgroundColor const neg_margin = rowEl.style.paddingLeft ? `-${rowEl.style.paddingLeft}` : "0" - const viewEl = div({style: {background_color: bg, margin_left: neg_margin, max_width: "100%", overflow_x: "hidden"}}) + const prev_child = rowEl.children[rowEl.children.length-1] + let viewEl + if (prev_child != null && prev_child.className == "row-content") { + viewEl = prev_child + } else { + viewEl = div({class: "row-content", style: {background_color: bg, margin_left: neg_margin, max_width: "100%", overflow_x: "hidden"}}) + } viewEl.appendChild(view.el) rowEl.appendChild(viewEl) if (!view.has_finished() && render) { @@ -851,7 +868,7 @@ export class DataTabulatorView extends HTMLBoxView { } else { const exp_index = expanded.indexOf(index) const removed = expanded.splice(exp_index, 1)[0] - const model = this.model.children.get(removed) + const model = this.get_child(removed) if (model != null) { const view = this._child_views.get(model) if (view !== undefined && view.el != null) { @@ -865,7 +882,7 @@ export class DataTabulatorView extends HTMLBoxView { } let ready = true for (const idx of this.model.expanded) { - if (this.model.children.get(idx) == null) { + if (this.get_child(idx) == null) { ready = false break } From bbde4cae577ffc1d9e9d12a07b48beba12f0805c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8xbro=20Hansen?= Date: Sat, 14 Sep 2024 17:02:07 +0200 Subject: [PATCH 2/4] Add isnumber check --- panel/models/tabulator.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index dcc3bbba7b..257cdb36cc 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -595,7 +595,9 @@ export class DataTabulatorView extends HTMLBoxView { return `${cell.getColumn().getField()}: ${cell.getValue()}` }) this.tabulator.on("scrollVertical", debounce(() => { - clearTimeout(this._scroll_timeout) + if (isNumber(this._scroll_timeout)) { + clearTimeout(this._scroll_timeout) + } this.record_scroll() this.setStyles() this._scroll_timeout = setTimeout(() => { From 6fd7899a9f73f694e6d9a0ab191e7d1e661d1693 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Sat, 14 Sep 2024 23:21:14 +0200 Subject: [PATCH 3/4] Improve scroll tracking and cleanup types --- panel/models/button.ts | 2 +- panel/models/checkbox_button_group.ts | 2 +- panel/models/html.ts | 3 +-- panel/models/icon.ts | 2 +- panel/models/radio_button_group.ts | 2 +- panel/models/tabulator.ts | 26 ++++++++++++++------------ 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/panel/models/button.ts b/panel/models/button.ts index 88bac84e38..9021e22d38 100644 --- a/panel/models/button.ts +++ b/panel/models/button.ts @@ -55,7 +55,7 @@ export class ButtonView extends BkButtonView { visible, }) } - let timer: number + let timer: ReturnType | undefined this.el.addEventListener("mouseenter", () => { timer = setTimeout(() => toggle(true), this.model.tooltip_delay) }) diff --git a/panel/models/checkbox_button_group.ts b/panel/models/checkbox_button_group.ts index bc7e88bb2b..0c8eeef6aa 100644 --- a/panel/models/checkbox_button_group.ts +++ b/panel/models/checkbox_button_group.ts @@ -58,7 +58,7 @@ export class CheckboxButtonGroupView extends bkCheckboxButtonGroupView { visible, }) } - let timer: number + let timer: ReturnType | undefined this.el.addEventListener("mouseenter", () => { timer = setTimeout(() => toggle(true), this.model.tooltip_delay) }) diff --git a/panel/models/html.ts b/panel/models/html.ts index d64a133181..53d7bc1fb1 100644 --- a/panel/models/html.ts +++ b/panel/models/html.ts @@ -73,7 +73,7 @@ export function run_scripts(node: Element): void { } function throttle(func: Function, limit: number): any { - let lastFunc: number + let lastFunc: ReturnType | undefined let lastRan: number return function(...args: any) { @@ -85,7 +85,6 @@ function throttle(func: Function, limit: number): any { lastRan = Date.now() } else { clearTimeout(lastFunc) - lastFunc = setTimeout(function() { if ((Date.now() - lastRan) >= limit) { func.apply(context, args) diff --git a/panel/models/icon.ts b/panel/models/icon.ts index e5467f3fb0..45af58693f 100644 --- a/panel/models/icon.ts +++ b/panel/models/icon.ts @@ -90,7 +90,7 @@ export class ClickableIconView extends ControlView { visible, }) } - let timer: number + let timer: ReturnType | undefined this.el.addEventListener("mouseenter", () => { timer = setTimeout(() => toggle_tooltip(true), this.model.tooltip_delay) }) diff --git a/panel/models/radio_button_group.ts b/panel/models/radio_button_group.ts index 59811eefb3..c18c184466 100644 --- a/panel/models/radio_button_group.ts +++ b/panel/models/radio_button_group.ts @@ -58,7 +58,7 @@ export class RadioButtonGroupView extends bkRadioButtonGroupView { visible, }) } - let timer: number + let timer: ReturnType | undefined this.el.addEventListener("mouseenter", () => { timer = setTimeout(() => toggle(true), this.model.tooltip_delay) }) diff --git a/panel/models/tabulator.ts b/panel/models/tabulator.ts index 257cdb36cc..7c6c6ce624 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -359,7 +359,6 @@ export class DataTabulatorView extends HTMLBoxView { _debounced_redraw: any = null _restore_scroll: boolean | "horizontal" | "vertical" = false _updating_scroll: boolean = false - _scroll_timeout: number | null = null _is_scrolling: boolean = false override connect_signals(): void { @@ -595,17 +594,7 @@ export class DataTabulatorView extends HTMLBoxView { return `${cell.getColumn().getField()}: ${cell.getValue()}` }) this.tabulator.on("scrollVertical", debounce(() => { - if (isNumber(this._scroll_timeout)) { - clearTimeout(this._scroll_timeout) - } - this.record_scroll() this.setStyles() - this._scroll_timeout = setTimeout(() => { - this._is_scrolling = true - }, 100) - }, 50, false)) - this.tabulator.on("scrollHorizontal", debounce(() => { - this.record_scroll() }, 50, false)) // Sync state with model @@ -659,10 +648,23 @@ export class DataTabulatorView extends HTMLBoxView { this.renderChildren() this.setStyles() + // Track scrolling position and active scroll + const holder = this.shadow_el.querySelector(".tabulator-tableholder") + let scroll_timeout: ReturnType | undefined + if (holder) { + holder.addEventListener('scroll', () => { + this.record_scroll() + this._is_scrolling = true + clearTimeout(scroll_timeout) + scroll_timeout = setTimeout(() => { + this._is_scrolling = false + }, 200) + }) + } + if (this.model.pagination) { if (this.model.page_size == null) { const table = this.shadow_el.querySelector(".tabulator-table") - const holder = this.shadow_el.querySelector(".tabulator-tableholder") if (table != null && holder != null) { const table_height = holder.clientHeight let height = 0 From 2de9de28c62bea0ab5eff6867d5a537d743f813d Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Sat, 14 Sep 2024 23:32:58 +0200 Subject: [PATCH 4/4] Fix lint --- 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 7c6c6ce624..bfdc4c2b63 100644 --- a/panel/models/tabulator.ts +++ b/panel/models/tabulator.ts @@ -652,7 +652,7 @@ export class DataTabulatorView extends HTMLBoxView { const holder = this.shadow_el.querySelector(".tabulator-tableholder") let scroll_timeout: ReturnType | undefined if (holder) { - holder.addEventListener('scroll', () => { + holder.addEventListener("scroll", () => { this.record_scroll() this._is_scrolling = true clearTimeout(scroll_timeout)