feat(webapp): render .csv/.tsv as a table instead of a wall of monospace (BEA-74) - #124
Open
ssowonny wants to merge 1 commit into
Open
feat(webapp): render .csv/.tsv as a table instead of a wall of monospace (BEA-74)#124ssowonny wants to merge 1 commit into
ssowonny wants to merge 1 commit into
Conversation
…ace (BEA-74) A .csv already previewed — as raw text in a <pre>, columns lining up only if the file happened to be padded. It now renders as an HTML table with the first row as a header. The parser is a new pure lib/csv.ts (~50 lines of RFC 4180: quoted delimiters, "" as a literal quote, newlines inside quotes), so no papaparse. It never throws: null means "not a table" — an unterminated quote, or a file with no delimiter at all — and the caller falls back to the very <pre> it renders today. That fallback is structural rather than a second code path, because TextView gained a `delim` prop instead of a new component: it also keeps the ["text", fileURL] query key a restore invalidates and the retry:false a pinned ?v= version needs. .tsv is new here — it used to fall through to SniffView and render as text. The delimiter comes from the extension, never from sniffing. Big files are capped at 5,000 rows with the count stated on screen (virtualization is out of scope). Wide files scroll inside .csvbox, whose rules are scoped under that class on purpose: the file pane carries .markdown, and the plain .markdown table rules — including the ≤900px one that turns a table into its own scroller — would otherwise out-specify a bare .csvview and give the page two nested scrollers. Not doing: sorting, filtering, search, editing, XLSX.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
.csvin the hub used to be a wall of monospaced text. It's a table now..tsvgets the same treatment — it's new, not a redirect: today a.tsvfalls through toSniffView.Before / after
Same file in both columns. It exercises every quoting case at once: a quoted
comma (
"Ortiz, Ana"stays one cell), a doubled quote (""ship it""rendersas
"ship it"), a newline inside quotes (LATAM's two-line cell is one row),and a short last row (
NAhas four fields where the header has five —the trailing cell renders empty instead of shifting its neighbours).
The row cap, stated on screen rather than silently applied:
The fallback is the feature
A viewer that white-screens on a malformed CSV is worse than the monospace it
replaces, so the fallback is structural rather than a
try/catchsomeone canforget.
parseDelimitedreturnsnull— never throws — andnullroutes tothe
<pre className="plain">that renders.csvtoday:That
<pre>is the same JSX in the same component, not a second code pathto keep in sync:
TextViewgained adelimprop instead of getting a siblingcomponent. It also inherits the two behaviours a new component would have had
to re-implement — the
["text", fileURL]query key a restore invalidates, andthe
retry: version ? false : undefineda pinned?v=<sha>version needs(so a
.csvopened from History is a table too).nullmeans one of two things, both intentional:What's in it
src/lib/csv.ts— a ~50-line character scanner over RFC 4180. No papaparse: the parser is smaller than the audit of a dependency would be. Pure, sonpm test(node's runner) can import it, same reason aslib/sniff.ts.util.ts—CSV_EXT = /\.(csv|tsv)$/i, checked beforeTEXT_EXT(which still listscsv; leaving it there is what makes the fallback path unchanged). Delimiter comes from the extension, never from sniffing the bytes.FileView.tsx— the new dispatch branch,TextView'sdelimprop, andCsvTable: a plain<table>, since with no sorting in scope@tanstack/react-tablewould be weight. Every row is padded to the widest one, so a ragged row in either direction renders correctly.style.css—.csvbox(overflow-x: auto,width: fit-content,max-width: 100%) and the.csvviewtreatment.The one CSS thing worth a look
Every CSV rule is scoped under
.csvbox, and that is load-bearing. The filepane carries
.markdown, so the plain.markdown table/th/tdrulesout-specify a bare
.csvview— including the@media (max-width: 900px)onethat turns any table into its own scroller. First cut had exactly that: two
nested scroll containers, and the outer one never moved. Measured at 390px, the
box now scrolls (
scrollWidth 2924vsclientWidth 352) and the page bodydoes not.
This also sidesteps the file conflict the plan flagged with BEA-70 — nothing
in the shared
@media (max-width: 900px)block was touched.Architecture changes
architecture/webapp-frontend.md: thelibclass gains one member,csv.ts parseDelimited Csv CSV_ROWS, and thecomponents --> libedge labelgains
parseDelimited. Nothing was removed and no seam moved —csv.tsis apeer of
sniff.tsanddiff.ts, not a new layer.flowchart TB components["<div style='text-align:left'><b>components</b><br/>FileView FolderListing FileTree<br/>HistoryView DiffView Insights<br/>...</div>"] lib["<div style='text-align:left'><b>lib</b> (pure, node-tested)<br/>+diff.ts splitLines lcsDiff diffText<br/>+runs.ts groupRuns runFileCount<br/>+heat.ts heatFor heatTotal heatLevel hotPathSplit<br/>+sniff.ts sniffBytes BlobText MAX_BYTES<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +csv.ts parseDelimited Csv CSV_ROWS</span><br/>+utils.ts</div>"] Note["csv.ts never throws.<br/>null = not a table<br/>(unterminated quote, no delimiter)<br/>and FileView falls back to pre.plain"] components -- "diffText groupRuns hotPathSplit placeLabels <span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅ parseDelimited</span>" --> lib lib -.- Note classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class Note noteBoxWhat was run
go build ./...,go vet ./...go test ./...npm testsrc/lib/csv.test.tsnpm run e2ebrowse.spec.tsinternal/webapp/staticNew e2e coverage: the quoting cases end to end,
.tsv, both fallback paths,a pinned
?v=version rendering as a table, the row-cap notice, and the 390pxscroll containment.
Deviations from the plan
One, small: the plan said to add
.csvboxto the shared@media (max-width: 900px)rule. Scoping the
.csvviewrules under.csvboxinstead was needed anyway(see above) and makes that line unnecessary, so the shared block is untouched.
Build session
(only works on the machine this ran on)