Skip to content

refactor(log-viewer): remove Tabulator - #1174

Merged
lcottercertinia merged 18 commits into
mainfrom
feat-remove-tabulator
Oct 16, 2026
Merged

lcottercertinia merged 18 commits into
mainfrom
feat-remove-tabulator

Conversation

@lukecotter

@lukecotter lukecotter commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #1169.

What changes

All 7 grids now use lv-grid, so this PR removes Tabulator:

  • The tabulator-tables and @types/tabulator-tables packages.
  • log-viewer/src/tabulator/. Its modules, renderer, group helpers, formatters, styles and their tests are deleted.
  • The Tabulator table code: TableShared.ts, CalltreeNameFormatter.ts, BottomUpTable.ts and ExpandCollapse.ts.
  • The Tabulator half of these files: locatedRow.ts (the row path stamp), CategoryColoring.ts (the row formatters), ColumnViews.ts, ColumnSettingsController.ts and GridColumnMenuController.ts.
  • The bench's Tabulator contender. The baseline JSON files stay, so compare.mjs still compares against them.
  • The lana/no-tabulator-html-tooltip lint rule.

The modules that lv-grid code still uses move to components/grid/: ColumnViews.ts, ColumnWidths.ts, MinMax.ts and measureWidth.ts. The Progress.css rules move into AppGrid's styles, the only grid that draws progress bars. LICENSE.txt no longer has the Tabulator notice, because the extension does not ship Tabulator now. The README acknowledgment says that earlier versions used it.

Not changed

The build still injects each stylesheet into document.head. Tabulator needed this for its popups on document.body. styles/tokens.css is now the only stylesheet, and its document copy is still needed as a fallback that shadow roots inherit (for example DatabaseMetricCard.ts reads a token without adopting the tokens). The comments say it is a fallback only, as .claude/rules/log-viewer.md does.

Follow-ups from /simplify and /code-review

  • The Inspector reads a grid row through a RowReader (data, parentOf), not through a Tabulator-shaped stand-in. inspectorRowEvents builds the reader from the data and parentOf options its callers already pass.
  • sass and the SCSS build support go: no .scss file is left.
  • The bench refuses c=tabulator, so a run cannot overwrite the Tabulator baseline.
  • The stale Tabulator comments are fixed.

Follow-ups from /deep-review

  • The grid bench refuses an unknown contender at once. It used to wait 10 minutes for tabulator.
  • The column menu takes its target once, and the column view names it a target, not a table.
  • The column menu reads the grid's own columns (id, title, hidden), with no Tabulator-shaped copy.
  • The Inspector's row reader is typed (RowData), and the inspector row options are the reader.
  • The SOQL group cell no longer keeps Tabulator's group-header signature.
  • The prettier globs drop scss. The rules keep the codicon font-size exemption, which now points at AppGrid.
  • A test checks every call-tree column view names a real column.

Commits

  1. refactor(log-viewer): remove Tabulator
  2. docs: drop the Tabulator notices and guidance
  3. refactor(log-viewer): read Inspector rows through a row reader
  4. build: remove the SCSS support
  5. chore: tidy what the Tabulator removal left
  6. fix(bench): refuse an unknown grid bench contender at once
  7. build: drop scss from the prettier globs
  8. docs(rules): keep the codicon font-size exemption, now in AppGrid
  9. docs(log-viewer): call the document token copy a fallback, as the rule does
  10. refactor(log-viewer): give the column menu its target once
  11. refactor(log-viewer): name a column view's targets as targets, not tables
  12. refactor(log-viewer): pass the inspector row options as the row reader
  13. refactor(log-viewer): type the row a reader gives the inspector
  14. test(log-viewer): check every call-tree view names a real column
  15. test(log-viewer): drop the rowFrames tests rowDetailSelection covers
  16. refactor(log-viewer): build the column menu from the grid's own columns
  17. refactor(log-viewer): build the SOQL group cell without Tabulator's header shape
  18. refactor(log-viewer): keep the progress bar styles in the grid that draws them

Not in this PR

  • Moving the column controllers into components/grid/.
  • Re-deriving ColumnWidths.ts for lv-grid. Its formula still uses Tabulator's sort-arrow reserve.

Tests

  • pnpm lint: pass
  • pnpm test: 2880/2880 pass
  • pnpm exec jest --selectProjects log-viewer: 2511/2511 pass. Main has 2627; the removed tests were Tabulator's or duplicates.
  • pnpm build: builds
  • pnpm test:e2e:web: 2/2 pass before the rebase onto main. It was not run after the rebase or the review commits. CI runs it.
  • Dev host: not checked after the review commits. The progress bars and the SOQL group header changed how they are styled.

@lukecotter
lukecotter force-pushed the feat-inspector-lv-grid branch from 96bd81f to ca9e923 Compare October 16, 2026 09:10
@lukecotter
lukecotter force-pushed the feat-remove-tabulator branch from 96ea2d0 to 1d8f846 Compare October 16, 2026 09:10
@lukecotter
lukecotter force-pushed the feat-inspector-lv-grid branch from ca9e923 to 606c4cd Compare October 16, 2026 14:55
Base automatically changed from feat-inspector-lv-grid to main October 16, 2026 15:46
Every grid uses lv-grid, so remove the tabulator-tables package, log-viewer/src/tabulator, the Tabulator table code, the bench's Tabulator contender and the Tabulator lint rule. The helpers that lv-grid code still uses move to components/grid.

Refs #1169
The extension no longer ships Tabulator, so the license notice goes, and the README acknowledgment says that earlier versions used it.
The Inspector read a grid row through a stand-in shaped like a Tabulator row. It now takes the row with a reader that gives its data and its parent.
No .scss file is left after the Tabulator removal, so sass and the SCSS build paths go.
Give the real reason the document copy of the tokens stays, and fix other comments that named Tabulator. The bench refuses c=tabulator, so a run cannot overwrite the Tabulator baseline, and drops its unused skip list.
@lukecotter
lukecotter force-pushed the feat-remove-tabulator branch from 1d8f846 to 41204e0 Compare October 16, 2026 15:53
@lcottercertinia
lcottercertinia merged commit fe34b17 into main Oct 16, 2026
10 checks passed
@lcottercertinia
lcottercertinia deleted the feat-remove-tabulator branch October 16, 2026 16:27
lcottercertinia pushed a commit that referenced this pull request Oct 16, 2026
…ctor call tree (#1182)

## Problem

With nothing selected, the Inspector call tree showed zero-duration
top-level lines as rows with no time, for example `USER_INFO`.

## Cause

The whole-log tree built each top-level event with `realSubtree`, which
keeps its root whatever its duration, as a selection is kept.

## Fix

`subtreeRoots` takes a `dropEmpty` option. Only the whole-log tree turns
it on: it leaves out a top-level row that has no duration and nothing
timed below it. A selection still always shows. The Call Tree tab still
shows `USER_INFO` as a row.

## Tests

- New: "leaves out a top-level line with no duration, as the Inspector
does for any row". It fails without the fix.
- `pnpm lint`: pass
- `pnpm test`: 2881/2881 pass, after merging main (#1174)

## CHANGELOG

No entry. The `USER_INFO` row is itself unreleased, so this most likely
never reached users.
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.

2 participants