Skip to content

Commit 898a10d

Browse files
authored
improvement(files): match CSV/XLSX preview tables to the markdown table chrome (#6125)
* improvement(files): match CSV/XLSX preview tables to the markdown table chrome Tables in the file viewer looked different depending on the file: CSV and XLSX previews rendered their own chrome (rounded outer frame, --surface-2 header, 13px body / 12px header, --text-secondary cells) while markdown files rendered tables through the rich markdown editor (full cell borders on --divider, --surface-4 header, 14px text). Extract the markdown table chrome into document-table.css and style both surfaces from it. The editor stylesheet keeps only its own concerns (fixed layout for column resizing, prose block margin, cell paragraph reset); DataTable keeps only its edit affordances. * fix(files): wrap unbreakable cell values in preview tables like markdown does The markdown prose root sets overflow-wrap: anywhere; the preview root did not, so with whitespace-nowrap gone a long URL or hash in a CSV cell would overflow instead of breaking.
1 parent 48aeac2 commit 898a10d

6 files changed

Lines changed: 220 additions & 29 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { forwardRef, memo, useCallback, useImperativeHandle, useRef, useState } from 'react'
44
import { cn } from '@sim/emcn'
5+
import './document-table.css'
56

67
interface EditConfig {
78
onCellChange: (row: number, col: number, value: string) => void
@@ -20,6 +21,11 @@ export interface DataTableHandle {
2021

2122
type EditingCell = { row: number; col: number } | null
2223

24+
/**
25+
* Tabular renderer for CSV and XLSX previews. Chrome (borders, padding, typography, header fill)
26+
* comes entirely from `document-table.css`, the definition shared with markdown tables in the rich
27+
* markdown editor — the only classes here are the optional edit affordances.
28+
*/
2329
const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataTable(
2430
{ headers, rows, editConfig },
2531
ref
@@ -94,16 +100,15 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
94100
editingCell?.row === row && editingCell?.col === col
95101

96102
return (
97-
<div className='overflow-x-auto rounded-md border border-[var(--border)]'>
98-
<table className='w-full border-collapse text-[13px]'>
99-
<thead className='bg-[var(--surface-2)]'>
103+
<div className='document-table overflow-x-auto'>
104+
<table>
105+
<thead>
100106
<tr>
101107
{headers.map((header, i) => (
102108
<th
103109
key={i}
104110
className={cn(
105-
'whitespace-nowrap px-3 py-2 text-left font-semibold text-[12px] text-[var(--text-primary)]',
106-
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-3)]'
111+
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-active)]'
107112
)}
108113
onClick={() => editConfig && startEdit(-1, i, String(header ?? ''))}
109114
>
@@ -114,7 +119,7 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
114119
onChange={(e) => setEditValue(e.target.value)}
115120
onBlur={commitEdit}
116121
onKeyDown={handleKeyDown}
117-
className='w-full min-w-[60px] bg-transparent font-semibold text-[12px] text-[var(--text-primary)] outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
122+
className='w-full min-w-[60px] bg-transparent outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
118123
/>
119124
) : (
120125
String(header ?? '')
@@ -125,13 +130,12 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
125130
</thead>
126131
<tbody>
127132
{rows.map((row, ri) => (
128-
<tr key={ri} className='border-[var(--border)] border-t'>
133+
<tr key={ri}>
129134
{headers.map((_, ci) => (
130135
<td
131136
key={ci}
132137
className={cn(
133-
'whitespace-nowrap px-3 py-2 text-[var(--text-secondary)]',
134-
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-2)]'
138+
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-active)]'
135139
)}
136140
onClick={() => editConfig && startEdit(ri, ci, String(row[ci] ?? ''))}
137141
>
@@ -142,7 +146,7 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
142146
onChange={(e) => setEditValue(e.target.value)}
143147
onBlur={commitEdit}
144148
onKeyDown={handleKeyDown}
145-
className='w-full min-w-[60px] bg-transparent text-[13px] text-[var(--text-secondary)] outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
149+
className='w-full min-w-[60px] bg-transparent outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
146150
/>
147151
) : (
148152
String(row[ci] ?? '')
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Canonical table chrome for the file viewer. Both surfaces that render a table for a file — the
3+
* rich markdown editor (`.rich-markdown-prose table`) and the tabular previews CSV/XLSX render
4+
* through `DataTable` (`.document-table`) — share this one definition so a table looks the same
5+
* whichever file it came from. Editor-only concerns (prose block margin, fixed layout for column
6+
* resizing, cell paragraph reset) stay in rich-markdown-editor.css.
7+
*/
8+
9+
/* `overflow-wrap` matches what `.rich-markdown-prose` sets on its own root: cells hold arbitrary
10+
file data, so an unbreakable token (a URL, a hash) must break rather than overflow its cell. */
11+
.document-table {
12+
color: var(--text-primary);
13+
overflow-wrap: anywhere;
14+
}
15+
16+
.rich-markdown-prose table,
17+
.document-table table {
18+
width: 100%;
19+
border-collapse: collapse;
20+
overflow: hidden;
21+
}
22+
23+
.rich-markdown-prose th,
24+
.rich-markdown-prose td,
25+
.document-table th,
26+
.document-table td {
27+
position: relative;
28+
border: 1px solid var(--divider);
29+
padding: 0.5rem 0.75rem;
30+
text-align: left;
31+
vertical-align: top;
32+
font-size: 14px;
33+
line-height: 1.5rem;
34+
}
35+
36+
.rich-markdown-prose th,
37+
.document-table th {
38+
background: var(--surface-4);
39+
font-weight: 600;
40+
}
41+
42+
/* Cell editors are invisible until focused: they take the cell's own typography and color. */
43+
.document-table input {
44+
font: inherit;
45+
color: inherit;
46+
}
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* A table must look the same whichever file it came from: a markdown file rendered by the rich
5+
* markdown editor (`.rich-markdown-prose table`) and a CSV/XLSX preview rendered by `DataTable`
6+
* (`.document-table`) sit in the same file viewer, in the same session, one click apart. They used
7+
* to drift — the previews carried their own chrome (rounded outer frame, `--surface-2` header,
8+
* 13px body / 12px header, `--text-secondary` cells) while markdown tables used full cell borders
9+
* on `--divider`, a `--surface-4` header, and 14px text.
10+
*
11+
* These load the real, shipped CSS (not a copy). Two complementary assertions, because jsdom's CSS
12+
* engine resolves only part of what matters here: it applies the cascade for longhand declarations
13+
* (so `getComputedStyle` parity is a real check on padding/typography/fill), but it does not expand
14+
* the `border` shorthand at all. Borders are therefore checked structurally — one rule, whose
15+
* selector list covers both roots — which is also the property that actually prevents drift.
16+
*/
17+
import { readFileSync } from 'node:fs'
18+
import path from 'node:path'
19+
import { afterEach, beforeAll, describe, expect, it } from 'vitest'
20+
21+
const SHARED_CSS_PATH = path.join(__dirname, 'document-table.css')
22+
const EDITOR_CSS_PATH = path.join(__dirname, 'rich-markdown-editor', 'rich-markdown-editor.css')
23+
24+
/** Chrome both surfaces must agree on, as longhands jsdom resolves. */
25+
const SHARED_CELL_PROPS = [
26+
'padding-top',
27+
'padding-right',
28+
'padding-bottom',
29+
'padding-left',
30+
'font-size',
31+
'line-height',
32+
'text-align',
33+
'vertical-align',
34+
] as const
35+
36+
const sheets = new Map<string, CSSStyleSheet>()
37+
38+
beforeAll(() => {
39+
for (const file of [SHARED_CSS_PATH, EDITOR_CSS_PATH]) {
40+
const style = document.createElement('style')
41+
style.textContent = readFileSync(file, 'utf-8')
42+
document.head.appendChild(style)
43+
if (!style.sheet) throw new Error(`stylesheet did not parse: ${file}`)
44+
sheets.set(file, style.sheet)
45+
}
46+
})
47+
48+
let containers: HTMLDivElement[] = []
49+
50+
afterEach(() => {
51+
for (const c of containers) c.remove()
52+
containers = []
53+
})
54+
55+
/** Mounts a one-cell table inside `rootClass` and returns the root plus its `th` and `td`. */
56+
function mountTable(rootClass: string): { root: HTMLElement; th: HTMLElement; td: HTMLElement } {
57+
const container = document.createElement('div')
58+
container.className = rootClass
59+
container.innerHTML =
60+
'<table><thead><tr><th>h</th></tr></thead><tbody><tr><td>c</td></tr></tbody></table>'
61+
document.body.appendChild(container)
62+
containers.push(container)
63+
const th = container.querySelector('th')
64+
const td = container.querySelector('td')
65+
if (!th || !td) throw new Error('table cells not found')
66+
return { root: container, th, td }
67+
}
68+
69+
function declarations(el: Element, props: readonly string[]): Record<string, string> {
70+
const computed = getComputedStyle(el)
71+
return Object.fromEntries(props.map((p) => [p, computed.getPropertyValue(p)]))
72+
}
73+
74+
/** Style rules of one loaded stylesheet, in source order. */
75+
function styleRules(cssPath: string): CSSStyleRule[] {
76+
const sheet = sheets.get(cssPath)
77+
if (!sheet) throw new Error(`stylesheet not loaded: ${cssPath}`)
78+
return Array.from(sheet.cssRules).filter((r): r is CSSStyleRule => r instanceof CSSStyleRule)
79+
}
80+
81+
/** Selector list of the single shared rule declaring `property: value`, as trimmed selectors. */
82+
function selectorsDeclaring(cssPath: string, property: string, value: string): string[] {
83+
const matching = styleRules(cssPath).filter((r) =>
84+
r.style.getPropertyValue(property).includes(value)
85+
)
86+
expect(matching).toHaveLength(1)
87+
return matching[0].selectorText.split(',').map((s) => s.trim())
88+
}
89+
90+
describe('document-table chrome is shared with markdown tables', () => {
91+
it('cells resolve to identical padding and typography', () => {
92+
const prose = mountTable('rich-markdown-prose')
93+
const preview = mountTable('document-table')
94+
95+
expect(declarations(preview.td, SHARED_CELL_PROPS)).toEqual(
96+
declarations(prose.td, SHARED_CELL_PROPS)
97+
)
98+
expect(declarations(preview.th, SHARED_CELL_PROPS)).toEqual(
99+
declarations(prose.th, SHARED_CELL_PROPS)
100+
)
101+
})
102+
103+
/**
104+
* Cells hold arbitrary file data, so an unbreakable token (a URL, a hash) must break rather than
105+
* overflow — the previews lost `whitespace-nowrap` and would otherwise have no wrapping rule at
106+
* all. `overflow-wrap` is inherited from each surface's root (jsdom does not propagate inherited
107+
* properties to descendants, so the roots are what can be asserted).
108+
*/
109+
it('both roots declare the same wrapping for unbreakable cell values', () => {
110+
const prose = mountTable('rich-markdown-prose')
111+
const preview = mountTable('document-table')
112+
113+
const wrap = getComputedStyle(prose.root).getPropertyValue('overflow-wrap')
114+
expect(wrap).toBe('anywhere')
115+
expect(getComputedStyle(preview.root).getPropertyValue('overflow-wrap')).toBe(wrap)
116+
})
117+
118+
it('the resolved values are the markdown editor values, not jsdom defaults', () => {
119+
const { th, td } = mountTable('document-table')
120+
121+
expect(getComputedStyle(td).getPropertyValue('padding-left')).toBe('0.75rem')
122+
expect(getComputedStyle(td).getPropertyValue('font-size')).toBe('14px')
123+
expect(getComputedStyle(th).getPropertyValue('font-weight')).toBe('600')
124+
})
125+
126+
it('one rule draws the cell border for both roots', () => {
127+
expect(selectorsDeclaring(SHARED_CSS_PATH, 'border', 'var(--divider)')).toEqual(
128+
expect.arrayContaining([
129+
'.rich-markdown-prose th',
130+
'.rich-markdown-prose td',
131+
'.document-table th',
132+
'.document-table td',
133+
])
134+
)
135+
})
136+
137+
it('one rule fills the header for both roots', () => {
138+
expect(selectorsDeclaring(SHARED_CSS_PATH, 'background', 'var(--surface-4)')).toEqual(
139+
expect.arrayContaining(['.rich-markdown-prose th', '.document-table th'])
140+
)
141+
})
142+
143+
it('the editor stylesheet no longer re-declares cell chrome of its own', () => {
144+
const redeclared = styleRules(EDITOR_CSS_PATH).filter(
145+
(r) =>
146+
/\.rich-markdown-prose (th|td)\b/.test(r.selectorText) &&
147+
(r.style.getPropertyValue('border') ||
148+
r.style.getPropertyValue('padding') ||
149+
r.style.getPropertyValue('font-size') ||
150+
r.style.getPropertyValue('background'))
151+
)
152+
153+
expect(redeclared.map((r) => r.selectorText)).toEqual([])
154+
})
155+
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -355,28 +355,12 @@
355355
border-radius: 4px;
356356
}
357357

358+
/* Borders, padding, typography, and header fill come from document-table.css — the chrome shared
359+
with the CSV/XLSX previews. Only the editor-specific bits live here: `table-layout: fixed` is
360+
required by prosemirror-tables' column-resizing plugin, and the block margin is prose rhythm. */
358361
.rich-markdown-prose table {
359-
width: 100%;
360-
border-collapse: collapse;
361362
table-layout: fixed;
362363
margin: 1rem 0;
363-
overflow: hidden;
364-
}
365-
366-
.rich-markdown-prose th,
367-
.rich-markdown-prose td {
368-
position: relative;
369-
border: 1px solid var(--divider);
370-
padding: 0.5rem 0.75rem;
371-
text-align: left;
372-
vertical-align: top;
373-
font-size: 14px;
374-
line-height: 1.5rem;
375-
}
376-
377-
.rich-markdown-prose th {
378-
background: var(--surface-4);
379-
font-weight: 600;
380364
}
381365

382366
.rich-markdown-prose th > p,

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { TableBubbleMenu } from './menus/table-menu'
3939
import { normalizeMarkdownContent } from './normalize-content'
4040
import { isRoundTripSafe } from './round-trip-safety'
4141
import '@sim/emcn/components/code/code.css'
42+
import '../document-table.css'
4243
import './rich-markdown-editor.css'
4344

4445
const EXTENSIONS = createMarkdownEditorExtensions({

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-field.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { LinkHoverCard } from './menus/link-hover-card'
1717
import { normalizeMarkdownContent } from './normalize-content'
1818
import { isRoundTripSafe } from './round-trip-safety'
1919
import '@sim/emcn/components/code/code.css'
20+
import '../document-table.css'
2021
import './rich-markdown-editor.css'
2122

2223
interface RichMarkdownFieldProps {

0 commit comments

Comments
 (0)