diff --git a/core/ui/modules/dnd.js b/core/ui/modules/dnd.js index fb1ca9e..52b8249 100644 --- a/core/ui/modules/dnd.js +++ b/core/ui/modules/dnd.js @@ -35,6 +35,18 @@ let lastHighlightedCell = null; function onDragOver(e) { e.preventDefault(); + + // Don't offer a drop target while a grid reload is in flight: the cells under + // the cursor are stale and about to be replaced. Leave dropEffect unset so the + // cursor shows "no-drop", and clear any lingering highlight. + if (state.isGridReloading) { + if (lastHighlightedCell) { + lastHighlightedCell.classList.remove('drag-over'); + lastHighlightedCell = null; + } + return; + } + e.dataTransfer.dropEffect = 'copy'; const cell = e.target.closest('.data-cell'); @@ -64,6 +76,11 @@ async function onDrop(e) { lastHighlightedCell = null; } + // Ignore drops while a grid reload is in flight: the targeted cell belongs to + // the stale result set about to be replaced, so the upload would land on the + // wrong row/column once the new data renders. + if (state.isGridReloading) return; + const cell = e.target.closest('.data-cell'); if (!cell || cell.classList.contains('row-number')) { return; diff --git a/core/ui/modules/grid-data.js b/core/ui/modules/grid-data.js index 72449d8..bfe1c46 100644 --- a/core/ui/modules/grid-data.js +++ b/core/ui/modules/grid-data.js @@ -38,22 +38,62 @@ export async function loadTableColumns() { } } +// Monotonic token identifying the most recent loadTableData() call. Concurrent +// loads (e.g. a slow fetch followed by a filter/sort/page change or a table +// switch, triggered via toolbar controls the grid guard doesn't cover) compare +// this after each await and bail if a newer load has started — so a superseded +// request never writes state, renders stale rows, shows a stale error, or clears +// the loading flag out from under the in-flight one. +let activeLoadToken = 0; + export async function loadTableData(showSpinner = true, saveScrollPosition = true) { if (!state.selectedTable) return; - const container = document.getElementById('gridContainer'); + const loadToken = ++activeLoadToken; + // Snapshot the target table/type for the whole request so an in-flight load + // can't pair this table's columns with a table the user switched to mid-fetch + // (which would SELECT the old columns against the new table and error out). + const requestedTable = state.selectedTable; + const requestedTableType = state.selectedTableType; + // This load is superseded if a newer load has started (token bumped) OR the + // user has navigated to a different table. The selection check matters because + // a table switch changes state.selectedTable synchronously but only starts its + // own load (bumping the token) after awaiting loadTableColumns(); during that + // gap the old load still owns the token, so a token-only check would let it + // render the old table's rows under the new selection and clear the flag. + const isSuperseded = () => loadToken !== activeLoadToken || requestedTable !== state.selectedTable; - // Only capture scroll position if the grid is currently visible (not loading/error state) - // This prevents overwriting the saved position with 0 when reloading data while a spinner is shown. - if (saveScrollPosition && container && container.querySelector('.data-grid')) { + const container = document.getElementById('gridContainer'); + // Whether a data grid is currently rendered (vs. a spinner/error/empty state), + // AND whether it belongs to the table we're loading. The renderedTable check is + // what separates a same-table refetch (filter/sort/page — keep the grid, no + // flicker) from a table switch, where the previous table's grid is still in the + // DOM and must not be left on screen. Cached once instead of re-querying below. + const hasRenderedGrid = !!(container && container.querySelector('.data-grid')); + const isSameTableGrid = hasRenderedGrid && state.renderedTable === state.selectedTable; + + // Only capture scroll position if the current table's grid is visible (not a + // loading/error state, and not a different table's grid mid-switch). This + // prevents overwriting the saved position with 0 while a spinner is shown. + if (saveScrollPosition && isSameTableGrid) { state.scrollPosition.left = container.scrollLeft; state.scrollPosition.top = container.scrollTop; } if (showSpinner) { state.isLoadingData = true; - showLoading(); + // Dedicated guard the grid handlers + global delete/select-all shortcuts + // key on. Separate from isLoadingData (also set by BLOB uploads in dnd.js) + // so the two can't clear each other; released by the latest load below. + state.isGridReloading = true; + // Keep the existing grid visible during a same-table refetch (prevents + // flicker); show the spinner on a true first load or a table switch, where + // nothing valid for this table is on screen yet. + if (!isSameTableGrid) { + showLoading(); + } } + updateToolbarButtons(); try { @@ -66,14 +106,19 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru } } + // Column names are needed both for the global-filter count and the data query. + const columnNames = state.tableColumns.map(c => c.name); + const countOptions = { filters, globalFilter: state.filterQuery, - columns: state.tableColumns.map(c => c.name) // Needed for global filter + columns: columnNames // Needed for global filter }; // Get total count - state.totalRecordCount = await backendApi.fetchTableCount(state.selectedTable, countOptions); + const totalRecordCount = await backendApi.fetchTableCount(requestedTable, countOptions); + if (isSuperseded()) return; // a newer load started, or the user switched tables + state.totalRecordCount = totalRecordCount; state.totalPageCount = Math.max(1, Math.ceil(state.totalRecordCount / state.rowsPerPage)); if (state.currentPageIndex >= state.totalPageCount) { @@ -81,8 +126,7 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru } // Get data - const isTable = state.selectedTableType === 'table'; - const columnNames = state.tableColumns.map(c => c.name); + const isTable = requestedTableType === 'table'; // For tables, we need to explicitly request the 'rowid' column to handle row identification. // The frontend expects rowid at index 0 for tables (see `getRowId` and `getRowDataOffset`). @@ -99,15 +143,20 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru globalFilter: state.filterQuery }; - const dataResult = await backendApi.fetchTableData(state.selectedTable, queryOptions); + const dataResult = await backendApi.fetchTableData(requestedTable, queryOptions); + if (isSuperseded()) return; // superseded (newer load or table switch) during the fetch state.gridData = dataResult.rows || []; - // If not showing spinner (background refresh), capture the current scroll position - // right before rendering. This ensures we use the latest scroll position, - // which covers cases where the user scrolled during fetch or if an edit operation - // updated the view (and restored scroll) while the fetch was pending. - if (!showSpinner && container && container.querySelector('.data-grid')) { + // When preserving scroll, re-capture the latest position right before + // rendering. Covers the user scrolling during the fetch — including a + // flicker-free refetch where the spinner was suppressed and the grid stayed + // interactive — and edit operations that restored scroll while the fetch was + // pending. Gated on saveScrollPosition (not !showSpinner) so callers that + // intentionally reset scroll (page change, table switch) aren't clobbered. + // Re-check the DOM here (not the cached flag): this runs after the await, + // so the rendered state may differ from when the function started. + if (saveScrollPosition && container && container.querySelector('.data-grid')) { state.scrollPosition.left = container.scrollLeft; state.scrollPosition.top = container.scrollTop; } @@ -119,6 +168,9 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru // updateCellDom in edit.js handles the visual update of the modified cell. } else { renderDataGrid(state.scrollPosition.top, state.scrollPosition.left); + // The on-screen grid now reflects this table; remember it so the next + // load can distinguish a same-table refetch from a table switch. + state.renderedTable = requestedTable; } if (container) { @@ -131,11 +183,23 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru } catch (err) { console.error('Error loading data:', err); - updateStatus(`Error: ${err.message}`); - showErrorState(err.message); + // Don't let a superseded load's error replace the current table's view. + if (!isSuperseded()) { + updateStatus(`Error: ${err.message}`); + showErrorState(err.message); + } } finally { + // isLoadingData keeps its original lifecycle. It is also set by BLOB uploads + // (dnd.js), so a superseded or no-spinner load must NOT clear it — only the + // spinner load that set it does, mirroring the pre-change behavior. if (showSpinner) { state.isLoadingData = false; } + // isGridReloading is owned solely here: the latest (non-superseded) load + // releases the grid-interaction guard once it settles, regardless of + // showSpinner. A superseded load leaves it set for the newer request to clear. + if (!isSuperseded()) { + state.isGridReloading = false; + } } } diff --git a/core/ui/modules/grid-events.js b/core/ui/modules/grid-events.js index e1140e5..d7c0b1a 100644 --- a/core/ui/modules/grid-events.js +++ b/core/ui/modules/grid-events.js @@ -52,6 +52,9 @@ function handleMousedown(event) { } function handleKeydown(event) { + // Ignore filter Enter while a load is in flight: acting now would queue a + // concurrent reload and operate against the stale, soon-to-be-replaced grid. + if (state.isGridReloading) return; if (event.target.classList.contains('column-filter')) { const colName = event.target.dataset.column; if (colName) onColumnFilterKeydown(event, colName); @@ -59,6 +62,11 @@ function handleKeydown(event) { } function handleClick(event) { + // Block grid selection/sort/filter/pin clicks while a load is in flight. The + // flicker fix keeps the previous grid visible during a same-table refetch, so + // without this guard a click on the stale row numbers or cells could select + // (and then delete) rows from the old result set before the new data arrives. + if (state.isGridReloading) return; const target = event.target; if (target.closest('.grid-header')) { handleHeaderClick(event, target); @@ -169,6 +177,8 @@ function handleBodyClick(event, target) { } function handleDoubleClick(event) { + // Don't open a cell editor on stale cells while a refetch is in flight. + if (state.isGridReloading) return; const cellEl = event.target.closest('.data-cell'); if (cellEl && !cellEl.classList.contains('row-number')) { const rowIdx = parseInt(cellEl.dataset.rowidx, 10); @@ -192,6 +202,13 @@ function handleMouseover(event) { } function handleScroll(event) { + // Ignore scroll while a load is in flight. The flicker fix keeps the grid + // mounted and scrollable during a refetch; without this, scrolling the old + // page during a page change (which reset scrollPosition to {0,0}) would + // overwrite the reset and reopen the new page at a stale offset. Same-table + // filter/sort refetches still preserve scroll via the post-await re-capture + // in loadTableData, which reads the live DOM position directly. + if (state.isGridReloading) return; const container = event.currentTarget; state.scrollPosition.left = container.scrollLeft; state.scrollPosition.top = container.scrollTop; diff --git a/core/ui/modules/state.js b/core/ui/modules/state.js index 29693ef..0aee203 100644 --- a/core/ui/modules/state.js +++ b/core/ui/modules/state.js @@ -7,6 +7,10 @@ export const state = { isDbConnected: false, selectedTable: null, selectedTableType: 'table', + // Name of the table whose grid is currently rendered on screen. Lets + // loadTableData tell a same-table refetch (keep the grid, no flicker) apart + // from a table switch (show the spinner instead of the previous table's rows). + renderedTable: null, currentPageIndex: 0, rowsPerPage: 500, totalRecordCount: 0, @@ -24,6 +28,12 @@ export const state = { activeCellInput: null, isSavingCell: false, isLoadingData: false, + // Dedicated guard for "a grid data reload is in flight", owned solely by + // loadTableData. Kept separate from isLoadingData (which BLOB uploads also set) + // so the grid-interaction guards can't be cleared by an unrelated upload. The + // grid event handlers and the global delete/select-all shortcuts key on this to + // avoid acting on rows that are about to be replaced. + isGridReloading: false, lastDoubleClickTime: 0, isTransitioningEdit: false, transitionLockTimeout: null, diff --git a/core/ui/viewer.html b/core/ui/viewer.html index 4a45d4a..55a82b6 100644 --- a/core/ui/viewer.html +++ b/core/ui/viewer.html @@ -364,7 +364,7 @@ diff --git a/core/ui/viewer.js b/core/ui/viewer.js index bf4c59d..6a3a56c 100644 --- a/core/ui/viewer.js +++ b/core/ui/viewer.js @@ -186,7 +186,10 @@ function setupGlobalShortcuts() { // Cmd+A / Ctrl+A if ((event.metaKey || event.ctrlKey) && event.key === 'a') { - if (state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; + // Bail during a grid reload: selecting "all" would capture row ids from + // the stale, about-to-be-replaced result set. This shortcut bypasses the + // #gridContainer handlers, so it needs its own guard. + if (state.isGridReloading || state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; if (state.selectedTable) { event.preventDefault(); @@ -196,7 +199,10 @@ function setupGlobalShortcuts() { // Delete / Backspace if ((event.metaKey || event.ctrlKey) && (event.key === 'Delete' || event.key === 'Backspace')) { - if (state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; + // Bail during a grid reload: deleting now would act on row/column/cell + // selections from the stale result set about to be replaced. Bypasses the + // #gridContainer handlers, so it needs its own guard. + if (state.isGridReloading || state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; if (state.selectedTable && state.selectedTableType === 'table') { event.preventDefault(); diff --git a/core/ui/web-viewer.js b/core/ui/web-viewer.js index e131e25..ae1cbaf 100644 --- a/core/ui/web-viewer.js +++ b/core/ui/web-viewer.js @@ -186,7 +186,9 @@ async function initializeApp() { // Cmd+A / Ctrl+A if ((event.metaKey || event.ctrlKey) && event.key === 'a') { - if (state.editingCellInfo || document.activeElement.tagName === 'INPUT') return; + // Bail during a grid reload: "select all" would capture row ids from + // the stale, about-to-be-replaced result set (mirrors core viewer.js). + if (state.isGridReloading || state.editingCellInfo || document.activeElement.tagName === 'INPUT') return; if (state.selectedTable) { event.preventDefault(); @@ -196,7 +198,9 @@ async function initializeApp() { // Delete / Backspace if ((event.metaKey || event.ctrlKey) && (event.key === 'Delete' || event.key === 'Backspace')) { - if (state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; + // Bail during a grid reload: deleting now would act on selections + // from the stale result set about to be replaced (mirrors viewer.js). + if (state.isGridReloading || state.editingCellInfo || document.activeElement.tagName === 'INPUT' || document.activeElement.tagName === 'TEXTAREA') return; if (state.selectedTable && state.selectedTableType === 'table') { event.preventDefault(); diff --git a/website/public/sqlite-viewer/viewer.html b/website/public/sqlite-viewer/viewer.html index a4add1a..aa96d82 100644 --- a/website/public/sqlite-viewer/viewer.html +++ b/website/public/sqlite-viewer/viewer.html @@ -364,7 +364,7 @@