From 39ffad8090b6aacfc1cdde505f707d8cf1e035a7 Mon Sep 17 00:00:00 2001 From: SamTV12345 <40429738+samtv12345@users.noreply.github.com> Date: Thu, 25 Jun 2026 22:04:47 +0200 Subject: [PATCH 1/2] fix(sheet): follow-up fixes for live presence (#311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delivers the fixes that landed after #311 was auto-merged: - vite: skip @rollup/plugin-commonjs for the sheet entry — it deadlocks the rolldown (vite 8) transform while bundling HyperFormula, so the sheet bundle never builds locally. - view: render the presence name tag via a CSS ::after pseudo-element (data-label) instead of a text node, so a peer's name no longer leaks into a cell's textContent (was e.g. "=A1*3anon"). - read-only: honor data.readonly on the client (non-editable cells), so no live-edit/commit frames originate from a viewer. - tests: two-session Playwright E2E for live presence/calculation, and a direct unit test for effectiveCells overlay-wins-on-collision. Co-Authored-By: Claude Opus 4.8 (1M context) --- playwright/specs/sheet_presence.spec.ts | 57 +++++++++++++++++++++++++ ui/src/js/sheet/sheetEditor.ts | 1 + ui/src/js/sheet/sheetPresence.test.ts | 10 +++++ ui/src/js/sheet/sheetView.ts | 10 ++++- ui/vite.config.ts | 8 ++-- 5 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 playwright/specs/sheet_presence.spec.ts diff --git a/playwright/specs/sheet_presence.spec.ts b/playwright/specs/sheet_presence.spec.ts new file mode 100644 index 00000000..0af59945 --- /dev/null +++ b/playwright/specs/sheet_presence.spec.ts @@ -0,0 +1,57 @@ +import { test, expect, type Page } from '@playwright/test'; + +// 0-based cell locator. Row header is th (child 1), so data column c is +// td:nth-child(c + 2); data row r is tbody tr:nth-child(r + 1). +const cell = (page: Page, r: number, c: number) => + page.locator(`.sheet-grid tbody tr:nth-child(${r + 1}) td:nth-child(${c + 2})`); + +async function openSheet(page: Page, padId: string): Promise { + await page.goto(`/s/${padId}`); + await page.locator('.sheet-grid').waitFor({ state: 'visible', timeout: 20000 }); +} + +async function commitCell(page: Page, r: number, c: number, text: string): Promise { + await cell(page, r, c).click(); + await page.keyboard.type(text, { delay: 30 }); + await page.keyboard.press('Enter'); +} + +test.describe('Sheet live presence & live calculation', () => { + test('cursor presence and live calculation across two sessions', async ({ browser }) => { + test.setTimeout(120000); + const padId = `sheet-presence-${Date.now()}`; + + // Session A sets up A1=10 and C2==B2+1. + const ctxA = await browser.newContext(); + const pageA = await ctxA.newPage(); + await openSheet(pageA, padId); + await commitCell(pageA, 0, 0, '10'); // A1 + await commitCell(pageA, 1, 2, '=B2+1'); // C2 + + // Session B joins and sees the committed value. + const ctxB = await browser.newContext(); + const pageB = await ctxB.newPage(); + await openSheet(pageB, padId); + await expect(cell(pageB, 0, 0)).toHaveText('10', { timeout: 20000 }); + + // A focuses B2 -> B sees a remote cursor tag on B2. + await cell(pageA, 1, 1).click(); + await expect(cell(pageB, 1, 1).locator('.sheet-remote-tag')).toBeVisible({ timeout: 20000 }); + + // A types a formula in B2 WITHOUT committing -> B sees the live formula text + // in B2 and C2 recomputed to 31, before Enter. + await pageA.keyboard.type('=A1*3', { delay: 50 }); + await expect(cell(pageB, 1, 1)).toHaveText('=A1*3', { timeout: 20000 }); + await expect(cell(pageB, 1, 2)).toHaveText('31', { timeout: 20000 }); + + // A commits -> B2 shows the computed result 30 (overlay replaced). + await pageA.keyboard.press('Enter'); + await expect(cell(pageB, 1, 1)).toHaveText('30', { timeout: 20000 }); + + // A disconnects -> A's cursor tag disappears on B (reused USER_LEAVE). + await ctxA.close(); + await expect(cell(pageB, 1, 1).locator('.sheet-remote-tag')).toHaveCount(0, { timeout: 20000 }); + + await ctxB.close(); + }); +}); diff --git a/ui/src/js/sheet/sheetEditor.ts b/ui/src/js/sheet/sheetEditor.ts index 74c90a2f..159d5215 100644 --- a/ui/src/js/sheet/sheetEditor.ts +++ b/ui/src/js/sheet/sheetEditor.ts @@ -122,6 +122,7 @@ export function startSheetEditor(root: HTMLElement): void { cols: 20, rawValue, displayValue, + readOnly: data.readonly, onEdit: (r, c, raw) => { if (!collab) return; collab.applyLocal({ type: 'setCell', sheet: activeSheetId, baseRev: collab.rev, row: r, col: c, raw }); diff --git a/ui/src/js/sheet/sheetPresence.test.ts b/ui/src/js/sheet/sheetPresence.test.ts index 64867332..1e3cd995 100644 --- a/ui/src/js/sheet/sheetPresence.test.ts +++ b/ui/src/js/sheet/sheetPresence.test.ts @@ -60,4 +60,14 @@ describe('effectiveCells overlay drives live recompute', () => { expect(engine.getValue(1, 2).value).toBe('31'); // C2 = B2+1 expect(cells.find((c) => c.row === 1 && c.col === 1)?.raw).toBe('=A1*3'); }); + + it('live edit wins over a committed cell at the same coordinate', () => { + // The load-bearing overlay property: a remote in-progress raw must REPLACE + // the committed raw at the same (row,col), not merely add another entry. + const base = [{ row: 1, col: 1, raw: '=A1*99' }]; + const live = [{ userId: 'a', name: 'A', color: '#f00', sheet: 's1', row: 1, col: 1, raw: '=A1*3' }]; + const cells = effectiveCells(base, live); + expect(cells.filter((c) => c.row === 1 && c.col === 1)).toHaveLength(1); + expect(cells.find((c) => c.row === 1 && c.col === 1)?.raw).toBe('=A1*3'); + }); }); diff --git a/ui/src/js/sheet/sheetView.ts b/ui/src/js/sheet/sheetView.ts index 697d45be..01d5f5a2 100644 --- a/ui/src/js/sheet/sheetView.ts +++ b/ui/src/js/sheet/sheetView.ts @@ -23,6 +23,7 @@ export interface SheetViewOptions { onSelect?: (row: number, col: number) => void; onLiveEdit?: (row: number, col: number, raw: string) => void; onEditEnd?: (row: number, col: number, committed: boolean) => void; + readOnly?: boolean; } function colName(c: number): string { @@ -44,6 +45,7 @@ const CSS = ` .sheet-grid td { outline: none; position: relative; } .sheet-grid td:focus { box-shadow: inset 0 0 0 2px #64d29b; } .sheet-remote-tag { position: absolute; top: -15px; left: -1px; font: 10px/14px system-ui, sans-serif; padding: 0 4px; color: #fff; border-radius: 3px 3px 3px 0; white-space: nowrap; z-index: 5; pointer-events: none; } +.sheet-remote-tag::after { content: attr(data-label); } `; export class DomSheetView { @@ -83,7 +85,9 @@ export class DomSheetView { const rowCells: HTMLTableCellElement[] = []; for (let c = 0; c < opts.cols; c++) { const td = document.createElement('td'); - td.contentEditable = 'true'; + // Read-only viewers get non-editable cells: with no typing, no live-edit + // or commit frames originate from the client (the server strips them too). + td.contentEditable = opts.readOnly ? 'false' : 'true'; this.attach(td, r, c); tr.appendChild(td); rowCells.push(td); @@ -174,7 +178,9 @@ export class DomSheetView { td.style.boxShadow = `inset 0 0 0 2px ${deco.color}`; const tag = document.createElement('span'); tag.className = 'sheet-remote-tag'; - tag.textContent = deco.name || 'anon'; + // Label via a CSS ::after pseudo-element (data-label), NOT a text node, + // so the peer's name never leaks into the cell's textContent. + tag.setAttribute('data-label', deco.name || 'anon'); tag.style.background = deco.color; td.appendChild(tag); this.decorated.add(td); diff --git a/ui/vite.config.ts b/ui/vite.config.ts index 8a062a99..6f5388c9 100644 --- a/ui/vite.config.ts +++ b/ui/vite.config.ts @@ -44,8 +44,10 @@ export default defineConfig(({ mode }) => { '@': path.resolve(__dirname, '/src'), }, }, - plugins: [ - commonjs(), - ], + // rolldown-vite (v8) handles CommonJS natively. The @rollup/plugin-commonjs + // plugin deadlocks the rolldown transform while bundling HyperFormula, so it + // is skipped for the sheet entry (rolldown's native CJS handling covers it). + // Other entries still load it unchanged. + plugins: mode === 'sheet' ? [] : [commonjs()], }; }); From 99729a41b20c4a20903c478fe9da4c8bd97a7355 Mon Sep 17 00:00:00 2001 From: SamTV12345 <40429738+samtv12345@users.noreply.github.com> Date: Thu, 25 Jun 2026 22:22:15 +0200 Subject: [PATCH 2/2] fix(build): build the sheet entry in build.js so CI/prod serve it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ui/build.js (the esbuild build used by CI's Playwright job and release builds) bundled pad/welcome/timeslider but never the sheet entry, so assets/js/sheet/ was empty and /s/:pad served no sheet.js — the spreadsheet grid never rendered outside a local vite build. Add the sheet esbuild entry (object form so the output is sheet.js, matching the path served by sheetFrontend.go). esbuild handles HyperFormula's CommonJS natively (no rolldown deadlock). Co-Authored-By: Claude Opus 4.8 (1M context) --- ui/build.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/build.js b/ui/build.js index b170b332..fad45836 100644 --- a/ui/build.js +++ b/ui/build.js @@ -334,6 +334,9 @@ const results = await Promise.allSettled([ esbuild.build({ ...esbuildOpts, entryPoints: ["./src/pad.js"], outdir: '../assets/js/pad/assets', alias, loader: loaders, logLevel: 'info' }), esbuild.build({ ...esbuildOpts, entryPoints: ["./src/welcome.js"], outdir: '../assets/js/welcome/assets', alias, loader: loaders, logLevel: 'info' }), esbuild.build({ ...esbuildOpts, entryPoints: ["./src/timeslider.js"], outdir: '../assets/js/timeslider/assets', alias, loader: loaders, logLevel: 'info' }), + // entryPoints object form so the output is sheet.js (matches /js/sheet/assets/sheet.js + // served by lib/api/pad/sheetFrontend.go), not sheet.entry.js. + esbuild.build({ ...esbuildOpts, entryPoints: { sheet: "./src/sheet.entry.ts" }, outdir: '../assets/js/sheet/assets', alias, loader: loaders, logLevel: 'info' }), // CSS bundles esbuild.build({ ...esbuildOpts, entryPoints: ['../assets/css/skin/colibris/pad.css'], outdir: '../assets/css/build/skin/colibris', external: ['*.woff', '*.woff2', '*.ttf', '*.eot', '*.svg', '*.png', '*.jpg', '*.gif'], loader: loaders, logLevel: 'info' }), esbuild.build({ ...esbuildOpts, entryPoints: ['../assets/css/static/pad.css'], outdir: '../assets/css/build/static', external: ['*.woff', '*.woff2', '*.ttf', '*.eot', '*.svg', '*.png', '*.jpg', '*.gif', '/font/*', 'font/*'], loader: loaders, logLevel: 'info' }),