Skip to content

Conversation

@Bowlerr
Copy link
Owner

@Bowlerr Bowlerr commented Dec 15, 2025

PR Title

Improve README (GIF demos) + table UX fixes (pagination, sort persistence, empty states, export)

Summary

This PR refreshes the extension README with GIF-driven docs and clarifies offline/security + optional CLI requirements, and improves table usability/reliability in the webview (pagination, sorting state, empty-state messaging, export correctness).

What Changed

Docs

  • README rewrite with "Offline + Secure" note, Quick Start, optional sqlite3/sqlcipher requirements, and embedded GIF demos.

UI/UX

  • Pagination controls now use delegated event handling so regenerated pagination HTML keeps working.
  • Per-tab client-side sort is persisted/restored (by column name).
  • Clear "No results on this page" empty states for page search/column filters (virtualized + non-virtual tables).
  • Search placeholder clarifies it searches the current page.
  • Page size options expanded for very large tables.
  • CSV export ignores non-data rows (empty-state + virtual spacer/loading rows).

Styling

  • New empty-state styling in tables.css.

Assets

  • Added demo GIFs under images.

Summary by CodeRabbit

  • Documentation

    • README refreshed with concise product overview, onboarding/GIF demos, Quick Start, streamlined commands/keybindings, compact troubleshooting, offline/security notes; CHANGELOG updated.
  • New Features

    • Larger pagination options (up to 100,000 rows).
    • Per‑tab sort state persisted and restored.
    • Improved empty‑state messaging and visual placeholders for no‑result filters.
    • Keyboard/focus improvements for pagination input and smoother pagination behaviour.
    • Context menu support on table cells.
  • Bug Fixes

    • CSV export now skips non‑data/placeholder rows.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Replaces the README with an offline-focused product overview and GIF gallery; adds per-tab sort state (with columnName) and persistence; centralises pagination via delegated handlers and keyboard/focus handling; introduces empty-state UI and placeholder rows; increases large page-size options and adjusts export/filter behaviour.

Changes

Cohort / File(s) Change Summary
Documentation
README.md, CHANGELOG.md
Rewritten README to an offline-first product onboarding and GIF showcase; simplified commands/troubleshooting/credits; added an Unreleased section to CHANGELOG describing README, sort persistence, empty-state messaging, pagination and export fixes.
Empty-state styling
media/css/30-components/tables.css
Added styles for .filter-empty-row, .table-empty-message, .table-empty-title, and .table-empty-description to render no-results / filter-empty UI.
Event delegation & pagination
media/events.js
Replaced per-button pagination listeners with a single delegated click handler on the table wrapper (initialised once); added Enter and focusout handlers for page-input; removed reattachment after DOM updates; added contextmenu hook attachment for cells; restored persisted sort state during view initialisation.
Per-tab view state
media/state.js
Added default sort in per-tab view state: { columnName: null, dir: "none" } (existing fields unchanged).
Table logic, sorting & persistence
media/table.js
Expanded PAGINATION_CONFIG.pageSizeOptions (added 10000, 100000), introduced optional columnName in virtual sort state and restore-by-name, resolved columnIndex from columnName when needed, persist/restored sort by name (try/catch guarded), added filter-empty placeholder rows and context-aware empty-state messages, updated search placeholder text, and excluded placeholders/virtual spacers from exports.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Webview as UI (table)
    participant Wrapper as TableWrapper
    participant Window as window handlers
    participant TableLogic as Table logic / Virtual renderer
    participant Storage as Tab-state storage

    User->>Webview: click pagination / press Enter on page-input
    Webview->>Wrapper: event bubbles (delegated)
    Wrapper->>Window: invoke handlePagination / updateTablePage
    Window->>TableLogic: request page change / sort change
    TableLogic->>Storage: persist sort/view state (columnName) [try/catch]
    TableLogic->>Webview: render rows (virtual or non-virtual)
    alt no-results
        TableLogic->>Webview: insert filter-empty-row + table-empty-message
    end
    Note over Window,TableLogic: contextmenu hooks attached to new cells during delta inserts
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas to focus on:
    • media/events.js: delegated click initialisation guard, Enter/focusout handling for page-input, removal of per-button reattachment, and contextmenu wiring.
    • media/table.js: sort persistence/restoration (columnName ↔ columnIndex resolution), virtual vs non-virtual sort paths, placeholder row lifecycle and export filtering.
    • media/state.js & restore ordering: interplay of pinned columns, widths, sort, scroll restoration and error handling around persistence calls.

Possibly related PRs

Poem

🐰 I hopped through tables, sorted by name,
Delegated clicks made the workflow tame,
Empty rows now whisper when searches fail,
Big pages hum while exports set sail,
Offline I nibble, and cheer the new frame.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Updated Readme and Persistent Sorting' directly addresses the two main changes in the PR: README refreshment with modern documentation and the addition of persistent sorting functionality across the extension.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df5aaf7 and c4b387d.

⛔ Files ignored due to path filters (3)
  • images/Database-Viewer-normal.gif is excluded by !**/*.gif
  • images/Tab-Organisation.gif is excluded by !**/*.gif
  • images/external-updates.gif is excluded by !**/*.gif
📒 Files selected for processing (1)
  • README.md (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
media/table.js (1)

661-707: Escape cell content in aria-label to avoid XSS via attributes

ariaValue is derived directly from cell and interpolated into the aria-label attribute without HTML escaping. If a cell contains quotes or markup‑like content, this can break the attribute and allow attackers (via crafted DB values) to inject additional attributes or event handlers into the <td>, which is effectively an XSS vector inside the webview.

You can mitigate this by escaping the dynamic portion before inserting it into the attribute, re‑using escapeHtmlFast:

-  const cellEditable = isEditable && !isBlob;
-  const ariaValue =
-    isBlob && blobBytes
-      ? `BLOB (${formatBytes(blobBytes.length)})`
-      : cell !== null
-      ? String(cell).substring(0, 50)
-      : "null";
+  const cellEditable = isEditable && !isBlob;
+  const rawAriaValue =
+    isBlob && blobBytes
+      ? `BLOB (${formatBytes(blobBytes.length)})`
+      : cell !== null && cell !== undefined
+      ? String(cell).substring(0, 50)
+      : "null";
+  const ariaValue = escapeHtmlFast(rawAriaValue);
@@
-                aria-label="Row ${globalIndex + 1}, Column ${
-    cellIndex + 1
-  }: ${ariaValue}${isForeignKey ? " (Foreign Key)" : ""}">
+                aria-label="Row ${globalIndex + 1}, Column ${
+    cellIndex + 1
+  }: ${ariaValue}${isForeignKey ? " (Foreign Key)" : ""}">

This keeps the ARIA description while ensuring hostile cell values cannot escape the attribute.

🧹 Nitpick comments (3)
media/events.js (2)

1424-1528: Sort restoration from tab view state works across virtual and non‑virtual tables

Restoring persisted sort by { columnName, dir } for both virtual (__virtualTableState) and non‑virtual tables is well structured: headers’ data-sort and indicators are reset before applying the saved sort, and rows are re‑ordered via compareValues with sensible fallbacks. This should make sort state feel reliably “sticky” per tab.

You are also restoring virtual sort from viewState inside initializeVirtualTable; if you want to trim work, you could rely on a single restoration path (either here or there) rather than both.


2395-2488: Delegated pagination events reduce re‑wiring overhead

The delegated click/keydown/focusout handlers on .enhanced-table-wrapper for .pagination-btn and .page-input are a good improvement: they survive regenerated pagination HTML and are guarded with data-pagination-delegated to avoid duplicate listeners. Logic for data-page, data-action="go", Enter, and focusout all correctly funnel into handlePagination/updateTablePage.

To avoid potentially calling updateTablePage twice when pressing Enter (keydown then focusout), you could short‑circuit the focusout path if the value has not changed since the last applied page.

media/table.js (1)

1338-1411: Non‑virtual filterTable correctly manages placeholder row and counts

The updated filterTable:

  • Normalises the wrapper and bails out if there is no <tbody>.
  • Removes any prior .filter-empty-row.
  • Inserts a single .filter-empty-row with the new empty‑state message when there are zero matches and a non‑empty search term.
  • Keeps the “Showing X of Y rows” label in sync while excluding the placeholder row from the total.

This matches the virtual path’s behaviour and avoids counting the placeholder as real data.

You might consider skipping rows with display: none when computing totalRows if you ever introduce non‑data helper rows that remain visible but are not logical data.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18bd175 and 03aed0c.

⛔ Files ignored due to path filters (10)
  • images/Blob-Viewer.gif is excluded by !**/*.gif
  • images/Database Viewer - normal.gif is excluded by !**/*.gif
  • images/Diagram.gif is excluded by !**/*.gif
  • images/Json-Viewer.gif is excluded by !**/*.gif
  • images/Query.gif is excluded by !**/*.gif
  • images/Relationation-Query.gif is excluded by !**/*.gif
  • images/Schema.gif is excluded by !**/*.gif
  • images/Tab Organisation.gif is excluded by !**/*.gif
  • images/encryption.gif is excluded by !**/*.gif
  • images/external updates.gif is excluded by !**/*.gif
📒 Files selected for processing (5)
  • README.md (1 hunks)
  • media/css/30-components/tables.css (1 hunks)
  • media/events.js (2 hunks)
  • media/state.js (1 hunks)
  • media/table.js (18 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{src/**/*.ts,media/**/*.js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{src/**/*.ts,media/**/*.js}: Use parameterized queries and input validation to prevent SQL injection in database operations
All database operations must be local with no network requests or telemetry

Files:

  • media/state.js
  • media/events.js
  • media/table.js
media/css/**/*.css

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

media/css/**/*.css: Use modular CSS in media/css/ with organized sections and component-based structure in subfolders like 30-components/
Always add new styles to the correct file and preserve import order in CSS
Use VS Code theme variables (--vscode-*) for all colors and backgrounds in CSS

Files:

  • media/css/30-components/tables.css
{src/databaseService.ts,media/table.js,media/main.js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use LIMIT/OFFSET for pagination, virtual scrolling for large tables, and clean up event listeners/resources

Files:

  • media/table.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Bowlerr/sqlite-intelliview-vscode PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-13T21:32:29.999Z
Learning: Applies to {src/databaseService.ts,media/table.js,media/main.js} : Use LIMIT/OFFSET for pagination, virtual scrolling for large tables, and clean up event listeners/resources
📚 Learning: 2025-12-13T21:32:29.999Z
Learnt from: CR
Repo: Bowlerr/sqlite-intelliview-vscode PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-13T21:32:29.999Z
Learning: Applies to {src/databaseService.ts,media/table.js,media/main.js} : Use LIMIT/OFFSET for pagination, virtual scrolling for large tables, and clean up event listeners/resources

Applied to files:

  • media/events.js
  • media/table.js
🧬 Code graph analysis (2)
media/events.js (1)
media/table.js (15)
  • viewState (866-869)
  • dir (928-931)
  • dir (1045-1045)
  • table (856-856)
  • table (1348-1348)
  • table (1831-1831)
  • vs (871-914)
  • vs (978-978)
  • vs (1340-1340)
  • vs (1421-1421)
  • vs (1706-1706)
  • vs (1802-1802)
  • idx (937-937)
  • cmp (1046-1052)
  • b (178-178)
media/table.js (3)
media/events.js (18)
  • totalPages (1249-1249)
  • totalPages (3050-3050)
  • pageSize (266-269)
  • pageSize (1010-1015)
  • pageSize (1244-1244)
  • pageSize (1402-1402)
  • pageSize (3046-3049)
  • pageSize (3144-3144)
  • viewState (1438-1439)
  • vs (1005-1008)
  • vs (1454-1454)
  • vs (3139-3139)
  • table (1452-1452)
  • table (1533-1533)
  • table (1583-1583)
  • table (2164-2166)
  • table (2197-2199)
  • table (2626-2626)
media/dom.js (1)
  • pageSize (243-250)
media/main.js (4)
  • vs (228-231)
  • vs (303-306)
  • vs (324-327)
  • table (660-660)
🪛 GitHub Actions: CI
README.md

[error] 1-1: Invalid image source in README.md: images/Database Viewer - normal.gif

🪛 LanguageTool
README.md

[grammar] ~137-~137: Normally, after “try to” a verb is expected.
Context: ...ase uses WAL mode, IntelliView will try to checkpoint the WAL before loading so you see up-to...

(WANT_TO_NN)


[style] ~178-~178: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...pped** (to keep the UI responsive). For very large exports, use the table UI’s CSV export ...

(EN_WEAK_ADJECTIVE)


[locale-violation] ~193-~193: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... ## Changelog See CHANGELOG.md. ## License MIT License – see LICENSE. ## Credi...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~195-~195: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...g See CHANGELOG.md. ## License MIT License – see LICENSE. ## Credits Built wit...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~195-~195: LICENSE must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...LOG.md. ## License MIT License – see LICENSE`. ## Credits Built with (bundled loca...

(LICENCE_LICENSE_NOUN_SINGULAR)

🔇 Additional comments (12)
media/state.js (1)

43-59: Default sort state addition looks sound

Adding sort: { columnName: null, dir: "none" } to the per‑tab view state is consistent with how getTabViewState merges defaults and how viewState.sort is read elsewhere (events.js/table.js). No behavioural regressions evident.

media/css/30-components/tables.css (1)

155-179: Empty-state styles align with new table messaging

The .filter-empty-row and .table-empty-* styles are consistent with the new empty-state rows/messages and correctly use VS Code theme variables. This should give a clear, unobtrusive “no results on this page” presentation without clashing with existing row hovers.

media/table.js (10)

83-99: Extended BLOB normalisation is sensible

Accepting plain numeric arrays as BLOBs in normalizeBlobValue (with the 0–255 guard) complements the existing Uint8Array, ArrayBuffer, and Node Buffer forms nicely and keeps the downstream rendering path unchanged.


296-341: Total pages calculation guarded by totalRowsKnown looks correct

Computing totalPages only when totalRowsKnown and falling back to 1 otherwise keeps pagination coherent when the backend has not yet reported a row count. Combined with visibleRowsLabel logic, this should avoid misleading “0 pages” edge cases.


386-389: Search placeholder accurately reflects scope

Changing the placeholder to Search table page... is a good UX clarification that this is a per‑page client‑side filter rather than a full‑table or database‑wide search.


871-943: Virtual table sort state initialisation matches new view-state shape

Using a richer vs.sort object with { columnName, columnIndex, dir } and seeding it from viewState.sort.columnName during initializeVirtualTable ensures virtual tables can restore sort by column name even if column indices shift between runs. This cooperates with recomputeVirtualMetrics and the persistence hooks elsewhere.


1008-1065: Column-name-aware sorting in recomputeVirtualMetrics is robust

Deriving columnIndex from vs.sort.columnName when needed and then sorting order via window.compareValues (or the localeCompare fallback) gives a stable, column‑name‑driven sort for virtual tables. The implementation correctly handles missing/unknown columns by simply not sorting.


1130-1164: Virtual empty-state message improves feedback for filtered/empty pages

The new virtualRender empty‑state, with table-empty-message and context‑sensitive title/description, is a nice UX improvement: it distinguishes between “no results on this page” versus “no rows on this page” and nudges users towards clearing filters or using the Query tab. It also reuses the existing empty-state styling you added in CSS.


1419-1483: Virtual sort toggling now persisted by column name

For virtualised tables, sortTableByColumn:

  • Updates header indicators and data-sort.
  • Stores vs.sort = { columnName, columnIndex, dir }.
  • Recomputes metrics and re-renders.
  • Persists { sort: { columnName, dir } } to the tab view state via setTabViewState.

This ensures sort survives tab switches/reloads using a stable key (column name) while still using the index for efficient sorting.


1492-1557: Non‑virtual sort persistence mirrors virtual behaviour

The non‑virtual branch now also persists sort per tab by { columnName, dir } after re‑ordering the DOM rows. This keeps behaviour consistent between small and virtualised tables, without changing the public sortTableByColumn signature.


1704-1794: Column-filter empty state and counts are consistent with global filtering

filterTableByColumn for non‑virtual tables now mirrors filterTable:

  • Cleans up any existing .filter-empty-row.
  • Adds a new placeholder row with clear guidance when a column filter hides all rows.
  • Updates .visible-rows while excluding the placeholder from the total.

This keeps the filtering UX coherent whether users filter by free‑text or by a specific column.


1800-1863: CSV export now skips non-data rows

Restricting visibleRows to tbody tr that are not hidden and not .filter-empty-row, .virtual-spacer, .virtual-loading, or .virtual-empty ensures CSV exports only include real data rows. This matches the UI expectation (“Export visible data”) and avoids exporting empty-state or spacer rows introduced by filtering/virtualisation.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
CHANGELOG.md (1)

15-15: Replace weak intensifier 'very' with stronger phrasing.

The phrase "very large page-size choices" uses an over-used intensifier. Consider replacing with "expanded" or "additional" for clearer, more direct phrasing.

Apply this diff:

-- 📄 **Bigger page size options**: Added very large page-size choices for browsing huge tables
+- 📄 **Bigger page size options**: Added expanded page-size choices for browsing huge tables

[Based on static analysis hints, LanguageTool.]

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03aed0c and 5176441.

📒 Files selected for processing (1)
  • CHANGELOG.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
CHANGELOG.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Update CHANGELOG.md with each release documenting breaking changes and migration instructions

Files:

  • CHANGELOG.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Bowlerr/sqlite-intelliview-vscode PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-13T21:32:29.999Z
Learning: Applies to {src/databaseService.ts,media/table.js,media/main.js} : Use LIMIT/OFFSET for pagination, virtual scrolling for large tables, and clean up event listeners/resources
📚 Learning: 2025-12-13T21:32:29.999Z
Learnt from: CR
Repo: Bowlerr/sqlite-intelliview-vscode PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-13T21:32:29.999Z
Learning: Applies to CHANGELOG.md : Update `CHANGELOG.md` with each release documenting breaking changes and migration instructions

Applied to files:

  • CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md

[style] ~15-~15: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... 📄 Bigger page size options: Added very large page-size choices for browsing huge tab...

(EN_WEAK_ADJECTIVE)

@Bowlerr Bowlerr merged commit a23d010 into main Dec 15, 2025
1 of 2 checks passed
@Bowlerr Bowlerr deleted the Persist-Sort branch January 21, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant