Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 86 additions & 12 deletions core/ui/modules/grid-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,56 @@ import { updateToolbarButtons } from './ui.js';
import { updateBatchSidebar } from './sidebar.js';
import { getRowId, getCellValue } from './data-utils.js';
import { openCellPreview, startCellEdit, openCellInVsCode } from './edit.js';

export function onFilterChange() {
clearTimeout(state.filterTimer);
state.filterTimer = setTimeout(() => {
state.filterQuery = document.getElementById('filterInput').value;
import { navigateMatches, resetMatchNav } from './match-nav.js';

/**
* Apply the global filter and jump to a match. The filter is only run when the
* user submits (Enter / Search button) — there is no filter-as-you-type. If the
* term is unchanged the grid already reflects it, so we skip the refetch and
* just advance to the next match.
*/
export async function applyGlobalFilter(direction = 1) {
// The toolbar filter input bypasses the #gridContainer guards, so block here
// while a reload is in flight to avoid a concurrent refetch / acting on the
// stale grid (the column filter is already covered by handleKeydown/handleClick).
if (state.isGridReloading) return;
const input = document.getElementById('filterInput');
if (!input) return;
const value = input.value;
if (value !== state.filterQuery) {
const previous = state.filterQuery;
state.filterQuery = value;
state.currentPageIndex = 0;
loadTableData();
resetMatchNav();
const ok = await loadTableData();
if (ok !== true) {
// Only a fully-applied load (true) should persist/navigate. false = a
// genuine failure: revert so the same query can be retried. undefined =
// superseded by a newer load (pagination/page-size/table switch); leave
// the term (that load is using it) and don't navigate against the stale
// grid while it's still in flight.
if (ok === false) state.filterQuery = previous;
return;
}
persistState();
}, 300);
}
navigateMatches('global', direction);
}
Comment on lines +17 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When applyGlobalFilter is registered as a click event listener on btnApplyFilter, the browser passes the MouseEvent object as the first argument (direction). This causes direction to be an object, leading to NaN when performing index calculations in navigateMatches. Additionally, we should defensively check if filterInput exists and prevent concurrent re-fetches if state.isLoadingData is true.

export async function applyGlobalFilter(direction = 1) {
    if (state.isLoadingData) return;
    const input = document.getElementById('filterInput');
    if (!input) return;
    const value = input.value;
    const dir = typeof direction === 'number' ? direction : 1;
    if (value !== state.filterQuery) {
        state.filterQuery = value;
        state.currentPageIndex = 0;
        resetMatchNav();
        await loadTableData();
        persistState();
    }
    navigateMatches('global', dir);
}


export function onFilterEnter(event) {
// Enter jumps to the next match, Shift+Enter to the previous one. Ignore the
// Enter that confirms an IME composition candidate (isComposing) so we don't
// submit the filter / preventDefault before the composed text is committed.
if (event.key === 'Enter' && !event.isComposing) {
event.preventDefault();
applyGlobalFilter(event.shiftKey ? -1 : 1);
}
}
Comment on lines +45 to 53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It is a good practice to call event.preventDefault() when handling the Enter keydown event to prevent any default browser actions (such as accidental form submissions or unexpected page reloads).

export function onFilterEnter(event) {
    // Enter jumps to the next match, Shift+Enter to the previous one.
    if (event.key === 'Enter') {
        event.preventDefault();
        applyGlobalFilter(event.shiftKey ? -1 : 1);
    }
}


export function onPageSizeChange() {
state.rowsPerPage = parseInt(document.getElementById('pageSizeSelect').value, 10);
state.currentPageIndex = 0;
resetMatchNav();
loadTableData();
persistState();
}
Expand All @@ -28,6 +64,9 @@ export function onDateFormatChange() {
const select = document.getElementById('dateFormatSelect');
if (select) {
state.dateFormat = select.value;
// Cached matches were computed against the previous formatted text, so they
// (and the highlighted active cell) are stale once the format changes.
resetMatchNav();
renderDataGrid();
persistState();
}
Expand All @@ -37,6 +76,7 @@ export function goToPage(pageIndex) {
if (pageIndex >= 0 && pageIndex < state.totalPageCount) {
state.currentPageIndex = pageIndex;
state.scrollPosition = { top: 0, left: 0 };
resetMatchNav();
loadTableData(true, false);
}
}
Expand All @@ -54,22 +94,56 @@ export function onColumnSort(columnName) {
state.sortedColumn = null;
state.sortAscending = true;
}
resetMatchNav();
loadTableData();
persistState();
}

export function applyColumnFilter(columnName) {
/**
* Apply a column filter and jump to a match. Like the global filter, this only
* runs on submit (Enter / Search button). When the term changed we refetch and
* restore focus to the (rebuilt) input so the user can keep pressing Enter to
* cycle through matches; when unchanged we just advance to the next match.
*/
export async function applyColumnFilter(columnName, direction = 1) {
if (state.isGridReloading) return; // don't stack a refetch/navigate on an in-flight reload
const input = document.querySelector(`.column-filter[data-column="${columnName}"]`);
if (input) {
if (!input) return;

const changed = input.value !== (state.columnFilters[columnName] || '');
if (changed) {
const previous = state.columnFilters[columnName];
state.columnFilters[columnName] = input.value;
state.currentPageIndex = 0;
loadTableData();
resetMatchNav();
const ok = await loadTableData();
if (ok !== true) {
// Only a fully-applied load (true) proceeds. false = genuine failure:
// restore the prior value so the query can be retried. undefined =
// superseded by a newer load; leave the value and don't navigate.
if (ok === false) {
if (previous === undefined) delete state.columnFilters[columnName];
else state.columnFilters[columnName] = previous;
}
return;
}
// loadTableData() rebuilds the header, so the input we focused is gone.
// Re-focus the freshly rendered one and place the caret at the end.
const newInput = document.querySelector(`.column-filter[data-column="${columnName}"]`);
if (newInput) {
newInput.focus();
newInput.setSelectionRange(newInput.value.length, newInput.value.length);
}
}
navigateMatches(columnName, direction);
}
Comment on lines +108 to 139

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To prevent race conditions and redundant database queries when the user rapidly presses Enter, we should check if state.isLoadingData is true and return early. We should also ensure direction is a number to prevent any potential type coercion issues.

export async function applyColumnFilter(columnName, direction = 1) {
    if (state.isLoadingData) return;
    const input = document.querySelector(`.column-filter[data-column="${columnName}"]`);
    if (!input) return;

    const dir = typeof direction === 'number' ? direction : 1;
    const changed = input.value !== (state.columnFilters[columnName] || '');
    if (changed) {
        state.columnFilters[columnName] = input.value;
        state.currentPageIndex = 0;
        resetMatchNav();
        await loadTableData();
        // loadTableData() rebuilds the header, so the input we focused is gone.
        // Re-focus the freshly rendered one and place the caret at the end.
        const newInput = document.querySelector(`.column-filter[data-column="${columnName}"]`);
        if (newInput) {
            newInput.focus();
            newInput.setSelectionRange(newInput.value.length, newInput.value.length);
        }
    }
    navigateMatches(columnName, dir);
}


export function onColumnFilterKeydown(event, columnName) {
if (event.key === 'Enter') {
applyColumnFilter(columnName);
// Enter jumps to the next match, Shift+Enter to the previous one. Ignore the
// IME composition-confirm Enter (isComposing) so CJK input isn't broken.
if (event.key === 'Enter' && !event.isComposing) {
event.preventDefault();
applyColumnFilter(columnName, event.shiftKey ? -1 : 1);
}
}
Comment on lines 141 to 148

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Call event.preventDefault() when handling the Enter keydown event to prevent default browser actions.

export function onColumnFilterKeydown(event, columnName) {
    // Enter jumps to the next match, Shift+Enter to the previous one.
    if (event.key === 'Enter') {
        event.preventDefault();
        applyColumnFilter(columnName, event.shiftKey ? -1 : 1);
    }
}


Expand Down
3 changes: 3 additions & 0 deletions core/ui/modules/grid-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,17 @@ export async function loadTableData(showSpinner = true, saveScrollPosition = tru

updatePagination();
updateStatus(`${state.totalRecordCount} records`);
return true; // signals callers (e.g. filter submit) that the load applied

} catch (err) {
console.error('Error loading data:', err);
// Don't let a superseded load's error replace the current table's view.
if (!isSuperseded()) {
updateStatus(`Error: ${err.message}`);
showErrorState(err.message);
return false; // the current load genuinely failed (lets callers retry)
}
// Superseded failure: a newer load owns the outcome — return undefined.
} 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
Expand Down
8 changes: 6 additions & 2 deletions core/ui/modules/grid-events.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { state, persistState } from './state.js';
import {
goToPage,
onFilterChange,
applyGlobalFilter,
onFilterEnter,
onPageSizeChange,
onDateFormatChange,
startColumnResize,
Expand All @@ -19,7 +20,10 @@ import {
import { openCellPreview } from './edit.js';

export function initGridControls() {
document.getElementById('filterInput')?.addEventListener('keyup', onFilterChange);
document.getElementById('filterInput')?.addEventListener('keydown', onFilterEnter);
// Wrap so the click MouseEvent isn't passed as `direction` (which would make
// navigateMatches compute NaN); the Search button always advances forward.
document.getElementById('btnApplyFilter')?.addEventListener('click', () => applyGlobalFilter(1));
document.getElementById('pageSizeSelect')?.addEventListener('change', onPageSizeChange);
document.getElementById('dateFormatSelect')?.addEventListener('change', onDateFormatChange);

Expand Down
19 changes: 15 additions & 4 deletions core/ui/modules/grid-render.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ function createTableHeader(rowNumWidth, orderedColumns, pinnedColumnOffsets) {
const keyIcon = col.isPrimaryKey ? '<span class="key-icon codicon codicon-key" title="Primary Key"></span>' : '';
const pinClass = isPinned ? 'pinned' : '';
const pinTitle = isPinned ? 'Unpin column' : 'Pin column';
const matchCounterText = state.matchNav.scope === col.name && state.matchNav.matches.length > 0
? `${state.matchNav.currentIndex + 1}/${state.matchNav.matches.length}`
: '';

th.innerHTML = `
<div class="header-content">
Expand All @@ -71,8 +74,11 @@ function createTableHeader(rowNumWidth, orderedColumns, pinnedColumnOffsets) {
<span class="pin-icon codicon codicon-pin ${pinClass}" title="${pinTitle}"></span>
</div>
<div class="header-bottom">
<input type="text" class="column-filter" data-column="${safeColName}" value="${safeFilterValue}" placeholder="Filter...">
<button class="filter-apply-btn" title="Apply filter (Enter)"><span class="codicon codicon-search"></span></button>
<div class="column-filter-wrap">
<input type="text" class="column-filter" data-column="${safeColName}" value="${safeFilterValue}" placeholder="Filter...">
<span class="column-filter-counter" data-column="${safeColName}">${matchCounterText}</span>
</div>
<button class="filter-apply-btn" title="Apply filter — Enter: next match, Shift+Enter: previous"><span class="codicon codicon-search"></span></button>
</div>
</div>
<div class="resize-handle"></div>
Expand All @@ -86,6 +92,10 @@ function createTableHeader(rowNumWidth, orderedColumns, pinnedColumnOffsets) {
function createTableBody(orderedColumns, columnIndexMap, pinnedColumnOffsets, rowNumWidth, headerHeight, rowHeight, selectedCellKeys, hasActiveFilters) {
const tbody = document.createElement('tbody');

const activeMatch = state.matchNav.currentIndex >= 0
? state.matchNav.matches[state.matchNav.currentIndex]
: null;

Comment on lines +96 to +98

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Clear stale match state before rendering new tables

This renders an active match solely from state.matchNav, but selecting another table clears filterQuery/columnFilters without resetting matchNav. After navigating matches in one table and then selecting a different table, the new grid can highlight the same row/column index even though the filter box is empty (and the old counter remains), so table switches show a stale match. Reset the match navigation state when table/filter state is cleared, or guard this active match on the current active term.

Useful? React with 👍 / 👎.

// Pinned rows logic
const pinnedRowsList = [];
for (let rowIdx = 0; rowIdx < state.gridData.length; rowIdx++) {
Expand Down Expand Up @@ -159,10 +169,11 @@ function createTableBody(orderedColumns, columnIndexMap, pinnedColumnOffsets, ro
const isColPinned = state.pinnedColumns.has(col.name);
const hasContent = !isNull && !(value instanceof Uint8Array);
const colWidth = state.columnWidths[col.name] || 120;
const isActiveMatch = !!activeMatch && activeMatch.rowIdx === rowIdx && activeMatch.colIdx === originalColIdx;

const td = document.createElement('td');
td.id = `cell-${rowIdx}-${originalColIdx}`;
td.className = `data-cell ${isNull ? 'null-value' : ''} ${isCellSelected ? 'cell-selected' : ''} ${isColPinned ? 'pinned' : ''}`;
td.className = `data-cell ${isNull ? 'null-value' : ''} ${isCellSelected ? 'cell-selected' : ''} ${isColPinned ? 'pinned' : ''} ${isActiveMatch ? 'active-match-cell' : ''}`;
td.dataset.rowidx = rowIdx;
td.dataset.colidx = originalColIdx;

Expand Down Expand Up @@ -208,7 +219,7 @@ function createTableBody(orderedColumns, columnIndexMap, pinnedColumnOffsets, ro
padding: '20px',
color: 'var(--text-secondary)'
});
td.textContent = 'No rows match the current filter. Modify or clear filters above.';
td.textContent = 'No rows match the current filter.';
tr.appendChild(td);
fragment.appendChild(tr);
}
Expand Down
119 changes: 119 additions & 0 deletions core/ui/modules/match-nav.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/**
* Filter Match Navigation
*
* Lets the user press Enter in the global filter or a column filter to jump
* between cells whose displayed text contains the active filter term,
* cycling through them with a visible border + a "current / total" counter.
*/
import { state } from './state.js';
import { getCellValue } from './data-utils.js';
import { formatCellValueAsText } from './utils.js';

function activeTerm(scope) {
return (scope === 'global' ? state.filterQuery : state.columnFilters[scope] || '').trim().toLowerCase();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Use a non-colliding scope for global search

When a table has a column literally named global, column-filter navigation for that column is treated as the toolbar global search because this sentinel is compared directly with column names. In that case activeTerm() reads state.filterQuery instead of state.columnFilters.global, and the matching code also scans every column, so Enter on the global column filter shows no/incorrect match navigation. Store the scope as a distinct type or reserve a sentinel that cannot collide with user column names.

Useful? React with 👍 / 👎.

}

function computeMatches(scope, term) {
const matches = [];
if (!term) return matches;

// Resolve the columns to scan and their data indices once, outside the row loop.
const columnsToScan = [];
state.tableColumns.forEach((col, idx) => {
if (scope === 'global' || col.name === scope) {
columnsToScan.push({ col, colIdx: idx });
}
});

for (let rowIdx = 0; rowIdx < state.gridData.length; rowIdx++) {
const row = state.gridData[rowIdx];
for (const { col, colIdx } of columnsToScan) {
const value = getCellValue(row, colIdx);
// String() guards against formatters that may return a non-string
// (number/null/undefined), which would otherwise throw on .toLowerCase().
const text = String(formatCellValueAsText(value, col.type, state.dateFormat, col.name));
if (text.toLowerCase().includes(term)) {
matches.push({ rowIdx, colIdx });
}
Comment on lines +31 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If formatCellValueAsText returns a non-string value (e.g., null, undefined, or a number), calling .toLowerCase() directly on it will throw a TypeError and crash the application. We should defensively convert the formatted value to a string.

            const value = getCellValue(row, colIdx);
            const rawText = formatCellValueAsText(value, col.type, state.dateFormat, col.name);
            const text = typeof rawText === 'string' ? rawText : String(rawText || '');
            if (text.toLowerCase().includes(term)) {
                matches.push({ rowIdx, colIdx });
            }

}
}
return matches;
}

function focusActiveMatch() {
document.querySelectorAll('.active-match-cell').forEach(el => el.classList.remove('active-match-cell'));

const { matches, currentIndex } = state.matchNav;
if (currentIndex < 0 || currentIndex >= matches.length) return;

const { rowIdx, colIdx } = matches[currentIndex];
const cellEl = document.getElementById(`cell-${rowIdx}-${colIdx}`);
if (cellEl) {
cellEl.classList.add('active-match-cell');
cellEl.scrollIntoView({ block: 'nearest', inline: 'nearest' });
}
}

function updateMatchCounterUI() {
const { scope, matches, currentIndex } = state.matchNav;
const counterText = matches.length > 0 ? `${currentIndex + 1}/${matches.length}` : '';

const globalCounter = document.getElementById('filterMatchCounter');
if (globalCounter) {
globalCounter.textContent = scope === 'global' ? counterText : '';
}

document.querySelectorAll('.column-filter-counter').forEach(el => {
el.textContent = el.dataset.column === scope ? counterText : '';
});
}

/**
* Move to the next (direction = 1) or previous (direction = -1) match for the
* given scope ('global' or a column name), wrapping around at either end.
* Matches are cached on `state.matchNav` and only recomputed when the scope or
* term changes (a fresh term is detected after `resetMatchNav()` cleared the
* cache), so pressing Enter/Shift+Enter repeatedly is O(matches), not a full
* rescan of every row.
*/
export function navigateMatches(scope, direction = 1) {
// Normalize to ±1 so a stray non-numeric arg (e.g. a DOM event) can never
// produce a NaN index in the modulo arithmetic below.
direction = direction < 0 ? -1 : 1;
const term = activeTerm(scope);
const cacheValid = state.matchNav.scope === scope
&& state.matchNav.term === term
&& state.matchNav.matches.length > 0;
Comment on lines +84 to +86

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Invalidate cached matches when displayed text changes

After matches are cached, this validity check only considers scope and term, but computeMatches() depends on the current rendered cell text via state.gridData and state.dateFormat. If a user edits a matched cell or changes the Date Format control and then presses Enter with the same filter term, navigation can keep jumping to/counting cells whose current text no longer matches. Include a data/display version in the cache key or reset match navigation when cell data or date formatting changes.

Useful? React with 👍 / 👎.


if (cacheValid) {
const len = state.matchNav.matches.length;
state.matchNav.currentIndex = (state.matchNav.currentIndex + direction + len) % len;
} else {
const matches = computeMatches(scope, term);
state.matchNav.scope = scope;
state.matchNav.term = term;
state.matchNav.matches = matches;
// Fresh search: forward starts at the first match, backward at the last.
state.matchNav.currentIndex = matches.length === 0
? -1
: (direction === 1 ? 0 : matches.length - 1);
}

focusActiveMatch();
updateMatchCounterUI();
}

/**
* Clear match navigation state, e.g. when the filter term or grid data
* changes outside of an explicit Enter-to-navigate action (sorting, paging,
* applying a new term) so a stale border/counter isn't left pointing at the
* wrong cell and the cache is invalidated.
*/
export function resetMatchNav() {
state.matchNav.scope = null;
state.matchNav.term = null;
state.matchNav.matches = [];
state.matchNav.currentIndex = -1;
document.querySelectorAll('.active-match-cell').forEach(el => el.classList.remove('active-match-cell'));
updateMatchCounterUI();
}
11 changes: 9 additions & 2 deletions core/ui/modules/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export const state = {
sortedColumn: null,
sortAscending: true,
filterQuery: '',
filterTimer: null,
selectedRowIds: new Set(),
gridData: [],

Expand Down Expand Up @@ -75,7 +74,15 @@ export const state = {

// Settings
dateFormat: 'raw', // 'raw', 'local', 'iso', 'relative'
cellEditBehavior: 'inline' // 'inline', 'modal', 'vscode'
cellEditBehavior: 'inline', // 'inline', 'modal', 'vscode'

// Filter match navigation (Enter-to-jump on global/column filters)
matchNav: {
scope: null, // 'global' or a column name
term: null, // the lowercased term the cached matches were computed for
matches: [], // [{ rowIdx, colIdx }] in row/column order
currentIndex: -1
}
};

/**
Expand Down
Loading