-
Notifications
You must be signed in to change notification settings - Fork 0
Updated Readme and Persistent Sorting #6
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces 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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
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. Comment |
There was a problem hiding this 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 inaria-labelto avoid XSS via attributes
ariaValueis derived directly fromcelland interpolated into thearia-labelattribute 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 tablesRestoring persisted sort by
{ columnName, dir }for both virtual (__virtualTableState) and non‑virtual tables is well structured: headers’data-sortand indicators are reset before applying the saved sort, and rows are re‑ordered viacompareValueswith sensible fallbacks. This should make sort state feel reliably “sticky” per tab.You are also restoring virtual sort from
viewStateinsideinitializeVirtualTable; 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 overheadThe delegated click/keydown/focusout handlers on
.enhanced-table-wrapperfor.pagination-btnand.page-inputare a good improvement: they survive regenerated pagination HTML and are guarded withdata-pagination-delegatedto avoid duplicate listeners. Logic fordata-page,data-action="go", Enter, and focusout all correctly funnel intohandlePagination/updateTablePage.To avoid potentially calling
updateTablePagetwice 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‑virtualfilterTablecorrectly manages placeholder row and countsThe updated
filterTable:
- Normalises the wrapper and bails out if there is no
<tbody>.- Removes any prior
.filter-empty-row.- Inserts a single
.filter-empty-rowwith 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: nonewhen computingtotalRowsif 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
⛔ Files ignored due to path filters (10)
images/Blob-Viewer.gifis excluded by!**/*.gifimages/Database Viewer - normal.gifis excluded by!**/*.gifimages/Diagram.gifis excluded by!**/*.gifimages/Json-Viewer.gifis excluded by!**/*.gifimages/Query.gifis excluded by!**/*.gifimages/Relationation-Query.gifis excluded by!**/*.gifimages/Schema.gifis excluded by!**/*.gifimages/Tab Organisation.gifis excluded by!**/*.gifimages/encryption.gifis excluded by!**/*.gifimages/external updates.gifis 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.jsmedia/events.jsmedia/table.js
media/css/**/*.css
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
media/css/**/*.css: Use modular CSS inmedia/css/with organized sections and component-based structure in subfolders like30-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.jsmedia/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 soundAdding
sort: { columnName: null, dir: "none" }to the per‑tab view state is consistent with howgetTabViewStatemerges defaults and howviewState.sortis 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 messagingThe
.filter-empty-rowand.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 sensibleAccepting plain numeric arrays as BLOBs in
normalizeBlobValue(with the 0–255 guard) complements the existingUint8Array,ArrayBuffer, and Node Buffer forms nicely and keeps the downstream rendering path unchanged.
296-341: Total pages calculation guarded bytotalRowsKnownlooks correctComputing
totalPagesonly whentotalRowsKnownand falling back to1otherwise keeps pagination coherent when the backend has not yet reported a row count. Combined withvisibleRowsLabellogic, this should avoid misleading “0 pages” edge cases.
386-389: Search placeholder accurately reflects scopeChanging 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 shapeUsing a richer
vs.sortobject with{ columnName, columnIndex, dir }and seeding it fromviewState.sort.columnNameduringinitializeVirtualTableensures virtual tables can restore sort by column name even if column indices shift between runs. This cooperates withrecomputeVirtualMetricsand the persistence hooks elsewhere.
1008-1065: Column-name-aware sorting inrecomputeVirtualMetricsis robustDeriving
columnIndexfromvs.sort.columnNamewhen needed and then sortingorderviawindow.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 pagesThe new
virtualRenderempty‑state, withtable-empty-messageand 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 nameFor 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 viasetTabViewState.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 behaviourThe 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 publicsortTableByColumnsignature.
1704-1794: Column-filter empty state and counts are consistent with global filtering
filterTableByColumnfor non‑virtual tables now mirrorsfilterTable:
- Cleans up any existing
.filter-empty-row.- Adds a new placeholder row with clear guidance when a column filter hides all rows.
- Updates
.visible-rowswhile 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 rowsRestricting
visibleRowstotbody trthat are not hidden and not.filter-empty-row,.virtual-spacer,.virtual-loading, or.virtual-emptyensures 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.
There was a problem hiding this 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
📒 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.mdwith 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)
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
UI/UX
Styling
Assets
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.