mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
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.
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
|
||||
import { forwardRef, memo, useCallback, useImperativeHandle, useRef, useState } from 'react'
|
||||
import { cn } from '@sim/emcn'
|
||||
import './document-table.css'
|
||||
|
||||
interface EditConfig {
|
||||
onCellChange: (row: number, col: number, value: string) => void
|
||||
@@ -20,6 +21,11 @@ export interface DataTableHandle {
|
||||
|
||||
type EditingCell = { row: number; col: number } | null
|
||||
|
||||
/**
|
||||
* Tabular renderer for CSV and XLSX previews. Chrome (borders, padding, typography, header fill)
|
||||
* comes entirely from `document-table.css`, the definition shared with markdown tables in the rich
|
||||
* markdown editor — the only classes here are the optional edit affordances.
|
||||
*/
|
||||
const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataTable(
|
||||
{ headers, rows, editConfig },
|
||||
ref
|
||||
@@ -94,16 +100,15 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
|
||||
editingCell?.row === row && editingCell?.col === col
|
||||
|
||||
return (
|
||||
<div className='overflow-x-auto rounded-md border border-[var(--border)]'>
|
||||
<table className='w-full border-collapse text-[13px]'>
|
||||
<thead className='bg-[var(--surface-2)]'>
|
||||
<div className='document-table overflow-x-auto'>
|
||||
<table>
|
||||
<thead>
|
||||
<tr>
|
||||
{headers.map((header, i) => (
|
||||
<th
|
||||
key={i}
|
||||
className={cn(
|
||||
'whitespace-nowrap px-3 py-2 text-left font-semibold text-[12px] text-[var(--text-primary)]',
|
||||
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-3)]'
|
||||
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-active)]'
|
||||
)}
|
||||
onClick={() => editConfig && startEdit(-1, i, String(header ?? ''))}
|
||||
>
|
||||
@@ -114,7 +119,7 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
|
||||
onChange={(e) => setEditValue(e.target.value)}
|
||||
onBlur={commitEdit}
|
||||
onKeyDown={handleKeyDown}
|
||||
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'
|
||||
className='w-full min-w-[60px] bg-transparent outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
|
||||
/>
|
||||
) : (
|
||||
String(header ?? '')
|
||||
@@ -125,13 +130,12 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
|
||||
</thead>
|
||||
<tbody>
|
||||
{rows.map((row, ri) => (
|
||||
<tr key={ri} className='border-[var(--border)] border-t'>
|
||||
<tr key={ri}>
|
||||
{headers.map((_, ci) => (
|
||||
<td
|
||||
key={ci}
|
||||
className={cn(
|
||||
'whitespace-nowrap px-3 py-2 text-[var(--text-secondary)]',
|
||||
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-2)]'
|
||||
editConfig && 'cursor-pointer select-none hover:bg-[var(--surface-active)]'
|
||||
)}
|
||||
onClick={() => editConfig && startEdit(ri, ci, String(row[ci] ?? ''))}
|
||||
>
|
||||
@@ -142,7 +146,7 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
|
||||
onChange={(e) => setEditValue(e.target.value)}
|
||||
onBlur={commitEdit}
|
||||
onKeyDown={handleKeyDown}
|
||||
className='w-full min-w-[60px] bg-transparent text-[13px] text-[var(--text-secondary)] outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
|
||||
className='w-full min-w-[60px] bg-transparent outline-none ring-1 ring-[var(--brand-secondary)] ring-inset'
|
||||
/>
|
||||
) : (
|
||||
String(row[ci] ?? '')
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
/*
|
||||
* Canonical table chrome for the file viewer. Both surfaces that render a table for a file — the
|
||||
* rich markdown editor (`.rich-markdown-prose table`) and the tabular previews CSV/XLSX render
|
||||
* through `DataTable` (`.document-table`) — share this one definition so a table looks the same
|
||||
* whichever file it came from. Editor-only concerns (prose block margin, fixed layout for column
|
||||
* resizing, cell paragraph reset) stay in rich-markdown-editor.css.
|
||||
*/
|
||||
|
||||
/* `overflow-wrap` matches what `.rich-markdown-prose` sets on its own root: cells hold arbitrary
|
||||
file data, so an unbreakable token (a URL, a hash) must break rather than overflow its cell. */
|
||||
.document-table {
|
||||
color: var(--text-primary);
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
.rich-markdown-prose table,
|
||||
.document-table table {
|
||||
width: 100%;
|
||||
border-collapse: collapse;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.rich-markdown-prose th,
|
||||
.rich-markdown-prose td,
|
||||
.document-table th,
|
||||
.document-table td {
|
||||
position: relative;
|
||||
border: 1px solid var(--divider);
|
||||
padding: 0.5rem 0.75rem;
|
||||
text-align: left;
|
||||
vertical-align: top;
|
||||
font-size: 14px;
|
||||
line-height: 1.5rem;
|
||||
}
|
||||
|
||||
.rich-markdown-prose th,
|
||||
.document-table th {
|
||||
background: var(--surface-4);
|
||||
font-weight: 600;
|
||||
}
|
||||
|
||||
/* Cell editors are invisible until focused: they take the cell's own typography and color. */
|
||||
.document-table input {
|
||||
font: inherit;
|
||||
color: inherit;
|
||||
}
|
||||
+155
@@ -0,0 +1,155 @@
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*
|
||||
* A table must look the same whichever file it came from: a markdown file rendered by the rich
|
||||
* markdown editor (`.rich-markdown-prose table`) and a CSV/XLSX preview rendered by `DataTable`
|
||||
* (`.document-table`) sit in the same file viewer, in the same session, one click apart. They used
|
||||
* to drift — the previews carried their own chrome (rounded outer frame, `--surface-2` header,
|
||||
* 13px body / 12px header, `--text-secondary` cells) while markdown tables used full cell borders
|
||||
* on `--divider`, a `--surface-4` header, and 14px text.
|
||||
*
|
||||
* These load the real, shipped CSS (not a copy). Two complementary assertions, because jsdom's CSS
|
||||
* engine resolves only part of what matters here: it applies the cascade for longhand declarations
|
||||
* (so `getComputedStyle` parity is a real check on padding/typography/fill), but it does not expand
|
||||
* the `border` shorthand at all. Borders are therefore checked structurally — one rule, whose
|
||||
* selector list covers both roots — which is also the property that actually prevents drift.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import { afterEach, beforeAll, describe, expect, it } from 'vitest'
|
||||
|
||||
const SHARED_CSS_PATH = path.join(__dirname, 'document-table.css')
|
||||
const EDITOR_CSS_PATH = path.join(__dirname, 'rich-markdown-editor', 'rich-markdown-editor.css')
|
||||
|
||||
/** Chrome both surfaces must agree on, as longhands jsdom resolves. */
|
||||
const SHARED_CELL_PROPS = [
|
||||
'padding-top',
|
||||
'padding-right',
|
||||
'padding-bottom',
|
||||
'padding-left',
|
||||
'font-size',
|
||||
'line-height',
|
||||
'text-align',
|
||||
'vertical-align',
|
||||
] as const
|
||||
|
||||
const sheets = new Map<string, CSSStyleSheet>()
|
||||
|
||||
beforeAll(() => {
|
||||
for (const file of [SHARED_CSS_PATH, EDITOR_CSS_PATH]) {
|
||||
const style = document.createElement('style')
|
||||
style.textContent = readFileSync(file, 'utf-8')
|
||||
document.head.appendChild(style)
|
||||
if (!style.sheet) throw new Error(`stylesheet did not parse: ${file}`)
|
||||
sheets.set(file, style.sheet)
|
||||
}
|
||||
})
|
||||
|
||||
let containers: HTMLDivElement[] = []
|
||||
|
||||
afterEach(() => {
|
||||
for (const c of containers) c.remove()
|
||||
containers = []
|
||||
})
|
||||
|
||||
/** Mounts a one-cell table inside `rootClass` and returns the root plus its `th` and `td`. */
|
||||
function mountTable(rootClass: string): { root: HTMLElement; th: HTMLElement; td: HTMLElement } {
|
||||
const container = document.createElement('div')
|
||||
container.className = rootClass
|
||||
container.innerHTML =
|
||||
'<table><thead><tr><th>h</th></tr></thead><tbody><tr><td>c</td></tr></tbody></table>'
|
||||
document.body.appendChild(container)
|
||||
containers.push(container)
|
||||
const th = container.querySelector('th')
|
||||
const td = container.querySelector('td')
|
||||
if (!th || !td) throw new Error('table cells not found')
|
||||
return { root: container, th, td }
|
||||
}
|
||||
|
||||
function declarations(el: Element, props: readonly string[]): Record<string, string> {
|
||||
const computed = getComputedStyle(el)
|
||||
return Object.fromEntries(props.map((p) => [p, computed.getPropertyValue(p)]))
|
||||
}
|
||||
|
||||
/** Style rules of one loaded stylesheet, in source order. */
|
||||
function styleRules(cssPath: string): CSSStyleRule[] {
|
||||
const sheet = sheets.get(cssPath)
|
||||
if (!sheet) throw new Error(`stylesheet not loaded: ${cssPath}`)
|
||||
return Array.from(sheet.cssRules).filter((r): r is CSSStyleRule => r instanceof CSSStyleRule)
|
||||
}
|
||||
|
||||
/** Selector list of the single shared rule declaring `property: value`, as trimmed selectors. */
|
||||
function selectorsDeclaring(cssPath: string, property: string, value: string): string[] {
|
||||
const matching = styleRules(cssPath).filter((r) =>
|
||||
r.style.getPropertyValue(property).includes(value)
|
||||
)
|
||||
expect(matching).toHaveLength(1)
|
||||
return matching[0].selectorText.split(',').map((s) => s.trim())
|
||||
}
|
||||
|
||||
describe('document-table chrome is shared with markdown tables', () => {
|
||||
it('cells resolve to identical padding and typography', () => {
|
||||
const prose = mountTable('rich-markdown-prose')
|
||||
const preview = mountTable('document-table')
|
||||
|
||||
expect(declarations(preview.td, SHARED_CELL_PROPS)).toEqual(
|
||||
declarations(prose.td, SHARED_CELL_PROPS)
|
||||
)
|
||||
expect(declarations(preview.th, SHARED_CELL_PROPS)).toEqual(
|
||||
declarations(prose.th, SHARED_CELL_PROPS)
|
||||
)
|
||||
})
|
||||
|
||||
/**
|
||||
* Cells hold arbitrary file data, so an unbreakable token (a URL, a hash) must break rather than
|
||||
* overflow — the previews lost `whitespace-nowrap` and would otherwise have no wrapping rule at
|
||||
* all. `overflow-wrap` is inherited from each surface's root (jsdom does not propagate inherited
|
||||
* properties to descendants, so the roots are what can be asserted).
|
||||
*/
|
||||
it('both roots declare the same wrapping for unbreakable cell values', () => {
|
||||
const prose = mountTable('rich-markdown-prose')
|
||||
const preview = mountTable('document-table')
|
||||
|
||||
const wrap = getComputedStyle(prose.root).getPropertyValue('overflow-wrap')
|
||||
expect(wrap).toBe('anywhere')
|
||||
expect(getComputedStyle(preview.root).getPropertyValue('overflow-wrap')).toBe(wrap)
|
||||
})
|
||||
|
||||
it('the resolved values are the markdown editor values, not jsdom defaults', () => {
|
||||
const { th, td } = mountTable('document-table')
|
||||
|
||||
expect(getComputedStyle(td).getPropertyValue('padding-left')).toBe('0.75rem')
|
||||
expect(getComputedStyle(td).getPropertyValue('font-size')).toBe('14px')
|
||||
expect(getComputedStyle(th).getPropertyValue('font-weight')).toBe('600')
|
||||
})
|
||||
|
||||
it('one rule draws the cell border for both roots', () => {
|
||||
expect(selectorsDeclaring(SHARED_CSS_PATH, 'border', 'var(--divider)')).toEqual(
|
||||
expect.arrayContaining([
|
||||
'.rich-markdown-prose th',
|
||||
'.rich-markdown-prose td',
|
||||
'.document-table th',
|
||||
'.document-table td',
|
||||
])
|
||||
)
|
||||
})
|
||||
|
||||
it('one rule fills the header for both roots', () => {
|
||||
expect(selectorsDeclaring(SHARED_CSS_PATH, 'background', 'var(--surface-4)')).toEqual(
|
||||
expect.arrayContaining(['.rich-markdown-prose th', '.document-table th'])
|
||||
)
|
||||
})
|
||||
|
||||
it('the editor stylesheet no longer re-declares cell chrome of its own', () => {
|
||||
const redeclared = styleRules(EDITOR_CSS_PATH).filter(
|
||||
(r) =>
|
||||
/\.rich-markdown-prose (th|td)\b/.test(r.selectorText) &&
|
||||
(r.style.getPropertyValue('border') ||
|
||||
r.style.getPropertyValue('padding') ||
|
||||
r.style.getPropertyValue('font-size') ||
|
||||
r.style.getPropertyValue('background'))
|
||||
)
|
||||
|
||||
expect(redeclared.map((r) => r.selectorText)).toEqual([])
|
||||
})
|
||||
})
|
||||
+3
-19
@@ -355,28 +355,12 @@
|
||||
border-radius: 4px;
|
||||
}
|
||||
|
||||
/* Borders, padding, typography, and header fill come from document-table.css — the chrome shared
|
||||
with the CSV/XLSX previews. Only the editor-specific bits live here: `table-layout: fixed` is
|
||||
required by prosemirror-tables' column-resizing plugin, and the block margin is prose rhythm. */
|
||||
.rich-markdown-prose table {
|
||||
width: 100%;
|
||||
border-collapse: collapse;
|
||||
table-layout: fixed;
|
||||
margin: 1rem 0;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.rich-markdown-prose th,
|
||||
.rich-markdown-prose td {
|
||||
position: relative;
|
||||
border: 1px solid var(--divider);
|
||||
padding: 0.5rem 0.75rem;
|
||||
text-align: left;
|
||||
vertical-align: top;
|
||||
font-size: 14px;
|
||||
line-height: 1.5rem;
|
||||
}
|
||||
|
||||
.rich-markdown-prose th {
|
||||
background: var(--surface-4);
|
||||
font-weight: 600;
|
||||
}
|
||||
|
||||
.rich-markdown-prose th > p,
|
||||
|
||||
+1
@@ -39,6 +39,7 @@ import { TableBubbleMenu } from './menus/table-menu'
|
||||
import { normalizeMarkdownContent } from './normalize-content'
|
||||
import { isRoundTripSafe } from './round-trip-safety'
|
||||
import '@sim/emcn/components/code/code.css'
|
||||
import '../document-table.css'
|
||||
import './rich-markdown-editor.css'
|
||||
|
||||
const EXTENSIONS = createMarkdownEditorExtensions({
|
||||
|
||||
+1
@@ -17,6 +17,7 @@ import { LinkHoverCard } from './menus/link-hover-card'
|
||||
import { normalizeMarkdownContent } from './normalize-content'
|
||||
import { isRoundTripSafe } from './round-trip-safety'
|
||||
import '@sim/emcn/components/code/code.css'
|
||||
import '../document-table.css'
|
||||
import './rich-markdown-editor.css'
|
||||
|
||||
interface RichMarkdownFieldProps {
|
||||
|
||||
Reference in New Issue
Block a user