-
Notifications
You must be signed in to change notification settings - Fork 6
Keep notebook Markdown within the visible pane #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c10794d
Keep notebook Markdown within the visible pane
jlewi 7708c60
Tighten scroll scope and browser review evidence
jlewi 13397b8
Align rendering CUJ with runner configuration and canonical docs
jlewi eecf6b4
Honor configured CUJ browser session and lifecycle options
jlewi 8d66f24
Preserve nested scroll focus when activating Markdown cells
jlewi 559a1f0
Use normal FFmpeg probing for CI browser recordings
jlewi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| /** CUJ: docs-dev/CUJs/horizontal-rendering.md. | ||
|
jlewi marked this conversation as resolved.
Outdated
|
||
| * Use stored output and synthetic Markdown: no runner or authenticated service | ||
| * is needed. Layout must be checked in Chromium; jsdom has no layout engine. | ||
| */ | ||
| import { execFileSync } from 'node:child_process' | ||
| import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' | ||
| import { dirname, join, resolve } from 'node:path' | ||
| import { fileURLToPath } from 'node:url' | ||
|
|
||
| const here = dirname(fileURLToPath(import.meta.url)) | ||
| const dir = here.endsWith('/.generated') ? dirname(here) : here | ||
| const output = join(dir, 'test-output') | ||
| const url = process.env.FRONTEND_URL ?? 'http://127.0.0.1:5173' | ||
|
jlewi marked this conversation as resolved.
Outdated
|
||
| const session = `horizontal-rendering-${Date.now()}` | ||
| const uri = 'local://file/horizontal-rendering-regression' | ||
| const fixture = JSON.parse( | ||
| readFileSync( | ||
| resolve(dir, '../fixtures/notebooks/horizontal-rendering.json'), | ||
| 'utf8' | ||
| ) | ||
| ) | ||
| mkdirSync(output, { recursive: true }) | ||
| let passed = 0 | ||
| let failed = 0 | ||
|
|
||
| /** Invoke the existing CUJ browser tool with argument boundaries preserved. */ | ||
| function browser(...args: string[]): string { | ||
| return execFileSync('agent-browser', ['--session', session, ...args], { | ||
| encoding: 'utf8', | ||
| timeout: 30000, | ||
|
jlewi marked this conversation as resolved.
|
||
| maxBuffer: 4 * 1024 * 1024, | ||
| }).trim() | ||
| } | ||
|
|
||
| /** Measure visible elements only: inactive tabs deliberately remain mounted. */ | ||
| function inspectLayout() { | ||
| const column = document.querySelector<HTMLElement>( | ||
| '[role="tabpanel"][data-state="active"] #notebook-column' | ||
| )! | ||
| const viewport = column.closest<HTMLElement>('.rt-ScrollAreaViewport')! | ||
| const markdown = column.querySelector<HTMLElement>('.notebook-markdown')! | ||
| const paragraph = markdown.querySelector<HTMLElement>('p')! | ||
| const tables = [...markdown.querySelectorAll('table')] | ||
| const pre = markdown.querySelector<HTMLElement>('pre')! | ||
| const output = column.querySelector<HTMLElement>( | ||
| '[data-testid="cell-output-item"] pre' | ||
| )! | ||
| const rect = paragraph.getBoundingClientRect() | ||
| const pane = viewport.getBoundingClientRect() | ||
| return { | ||
| viewport: viewport.clientWidth, | ||
| column: column.clientWidth, | ||
| scroll: viewport.scrollWidth, | ||
| proseFits: rect.left >= pane.left && rect.right <= pane.right + 1, | ||
| proseWraps: | ||
| rect.height > parseFloat(getComputedStyle(paragraph).lineHeight) * 1.5, | ||
| smallTableFits: | ||
| tables[0].scrollWidth <= tables[0].parentElement!.clientWidth + 1, | ||
| wideTableScrolls: | ||
| tables[1].parentElement!.scrollWidth > | ||
| tables[1].parentElement!.clientWidth && | ||
| getComputedStyle(tables[1].parentElement!).overflowX === 'auto', | ||
| codeScrolls: | ||
| pre.scrollWidth > pre.clientWidth && | ||
| getComputedStyle(pre).overflowX === 'auto', | ||
| scrollRegionsFocusable: | ||
| pre.tabIndex === 0 && tables[1].parentElement!.tabIndex === 0, | ||
| outputFits: output.scrollWidth <= output.clientWidth + 1, | ||
| outputIntact: output.textContent!.includes( | ||
| 'unbroken_output_identifier_'.repeat(500) | ||
| ), | ||
| siblingFits: | ||
| column.querySelectorAll('.notebook-markdown').length === 2 && | ||
| [...column.querySelectorAll<HTMLElement>('.notebook-markdown')].every( | ||
| (cell) => { | ||
| const bounds = cell.getBoundingClientRect() | ||
| return bounds.left >= pane.left && bounds.right <= pane.right + 1 | ||
| } | ||
| ), | ||
| } | ||
| } | ||
|
|
||
| /** Fail on missing elements rather than treating empty measurements as success. */ | ||
| function check(name: string, ok: boolean) { | ||
| if (ok) passed++ | ||
| else failed++ | ||
| console.log(`[${ok ? 'PASS' : 'FAIL'}] ${name}`) | ||
| } | ||
|
|
||
| try { | ||
| browser('open', url) | ||
| browser('record', 'start', join(output, 'scenario-horizontal-rendering.webm')) | ||
| // Recording may replace the context. Seed only after recording has started. | ||
| browser('wait', '--fn', 'Boolean(window.app?.localNotebooks)') | ||
| browser( | ||
| 'eval', | ||
| `(async () => { | ||
| const store = window.app?.localNotebooks; | ||
| if (!store) throw new Error('Local notebook store is not ready'); | ||
| await store.files.put({ | ||
| id:${JSON.stringify(uri)}, uri:${JSON.stringify(uri)}, | ||
| name:'horizontal-rendering.json', remoteId:'', | ||
| doc:${JSON.stringify(JSON.stringify(fixture))}, | ||
| parent:'local://folder/local', updatedAt:new Date().toISOString(), | ||
| lastSynced:'', lastRemoteChecksum:'' | ||
| }); | ||
| sessionStorage.setItem('runme/openNotebooks',JSON.stringify([{uri:${JSON.stringify(uri)},name:'horizontal-rendering.json',type:'file',children:[],parents:['local://folder/local']} ])); | ||
| sessionStorage.setItem('runme/currentDoc',${JSON.stringify(uri)}); | ||
| })()` | ||
| ) | ||
| browser('reload') | ||
| browser('wait', '#markdown-rendered-markup_horizontal_rendering') | ||
| browser('wait', '[data-testid="cell-output-item"] pre') | ||
| browser('wait', '--fn', 'document.fonts.status === "loaded"') | ||
| // Exercise a normal and a narrow viewport; the sidebar may consume width too. | ||
| for (const width of [1280, 900]) { | ||
| browser('set', 'viewport', String(width), '900') | ||
| browser('wait', '300') | ||
| const raw = browser('eval', `(${inspectLayout.toString()})()`) | ||
| let result = JSON.parse(raw) | ||
| if (typeof result === 'string') result = JSON.parse(result) | ||
| writeFileSync( | ||
| join(output, `scenario-horizontal-rendering-${width}.json`), | ||
| JSON.stringify(result, null, 2) | ||
| ) | ||
| check( | ||
| `${width}: notebook width is bounded`, | ||
| result.column <= result.viewport + 1 && | ||
| result.scroll <= result.viewport + 1 | ||
| ) | ||
| for (const key of [ | ||
| 'proseFits', | ||
| 'proseWraps', | ||
| 'smallTableFits', | ||
| 'wideTableScrolls', | ||
| 'codeScrolls', | ||
| 'scrollRegionsFocusable', | ||
| 'outputFits', | ||
| 'outputIntact', | ||
| 'siblingFits', | ||
| ]) | ||
| check(`${width}: ${key}`, result[key] === true) | ||
| browser( | ||
| 'screenshot', | ||
| join(output, `scenario-horizontal-rendering-${width}.png`) | ||
| ) | ||
| } | ||
| } catch (error) { | ||
| check(String(error), false) | ||
| } finally { | ||
| try { | ||
| browser('record', 'stop') | ||
| } catch { | ||
| /* The first failure is reported above. */ | ||
| } | ||
| try { | ||
| browser('close') | ||
| } catch { | ||
| /* Preserve test result during cleanup. */ | ||
| } | ||
| } | ||
| console.log( | ||
| `Assertions: ${passed + failed}, Passed: ${passed}, Failed: ${failed}` | ||
| ) | ||
| process.exitCode = failed ? 1 : 0 | ||
Large diffs are not rendered by default.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # Read notebooks containing wide Markdown and outputs | ||
|
|
||
| Regression: a wide table, code block, or output expands Radix's fit-content | ||
| wrapper and makes every paragraph wrap beyond the visible pane. | ||
|
|
||
| ## Design decisions | ||
|
|
||
| - The viewport owns notebook width. Constrain Radix's generated inner wrapper | ||
| to 100% width and max-width, scoped to `.notebook-scroll-area`. `w-full` on | ||
| the notebook alone refers to the already expanded wrapper and cannot fix it. | ||
| - The notebook scrolls vertically. Horizontal scrolling belongs to individual | ||
| tables and fenced code blocks, with keyboard-focusable scroll regions. | ||
| - Prose, links, inline identifiers and plain-text outputs use `overflow-wrap: | ||
| anywhere`. Unlike `break-word`, it also reduces intrinsic minimum width. | ||
| - Tables use normal wrapping so wide columns stay readable and scroll locally. | ||
| Fenced code preserves whitespace; the `pre`, not a second nested `code` | ||
| scroller, owns overflow. Do not clip content to conceal a sizing regression. | ||
| - Scope the unlayered CSS override to notebook panes; other Radix scroll areas, | ||
| including the tab rail and review panels, keep their existing behavior. | ||
|
|
||
| Full design and before/after evidence: | ||
| [20260910_horizontal_rendering.runme](https://runme.gateway.unified-0.internal.api.openai.org/?doc=https%3A%2F%2Fdrive.google.com%2Ffile%2Fd%2F1m4Zx1EPXjrrXKYdzgXBhTatI5nYLj-ES%2Fview). | ||
|
|
||
| ## Fixture and acceptance criteria | ||
|
|
||
| `app/test/fixtures/notebooks/horizontal-rendering.json` includes ordinary prose, | ||
| a two-column table, long inline code, a long fenced command, a 12-column table, | ||
| a long stored output URL, and a sibling Markdown cell. It contains no real | ||
| incident data or service credentials and needs no backend execution. | ||
|
|
||
| Run `test-scenario-horizontal-rendering.ts` through the canonical CUJ runner. | ||
| At 1280px and 900px browser widths, require: | ||
|
|
||
| 1. Notebook column and scroll width do not exceed the viewport (1px tolerance). | ||
| 2. Paragraphs fit and wrap; the small table fits. | ||
| 3. Wide table and code block overflow locally, with keyboard focus available. | ||
| 4. Long output remains intact and wraps within its output panel. | ||
| 5. The sibling cell is present; earlier wide content does not expand the column. | ||
|
|
||
| Record measurements, screenshots and video under `app/test/browser/test-output/`. | ||
| Also inspect the wide blocks after scrolling locally and test resizing with | ||
| Explorer/Comments panels open. A CSS-class test in jsdom cannot establish these | ||
| layout properties; use browser geometry and inspect screenshots. |
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.