BL-16818 Tables 1/6: a table you can put in an origami section - #8325
Open
hatton wants to merge 2 commits into
Open
BL-16818 Tables 1/6: a table you can put in an origami section#8325hatton wants to merge 2 commits into
hatton wants to merge 2 commits into
Conversation
First of six PRs splitting BL-16818 (PR 8315) into reviewable pieces. This one lands enough to try the feature: turn on Tables in the collection's Advanced settings, then choose Table in an origami section's type chooser and edit it. Depend on bloom-table. It is not on npm and is not going to be; it ships as a committed-dist tag on GitHub, the same mechanism bloom-ai-image-tools uses, so package.json pins github:BloomBooks/bloom-table#dist-v1.0.0. basePage.less inlines the library's stylesheet from node_modules. Vitest needs test.server.deps.inline for it, because the library's ESM build imports MUI by subpath and MUI 5 ships no "exports" map, so Node's loader rejects those as directory imports. Gate the feature at the Pro tier behind a "tables" experiment: FeatureRegistry gains FeatureName.Table with PreventPublishingInOriginalBooks, and ExperimentalFeatures gains kTables, surfaced by the Advanced tab checkbox. tableEditing.ts attaches a table, wires each cell's translation group for editing (including cells the library builds after page load, and nested tables), and tears the listeners down again. Undo has two stacks now, the table's and CKEditor's, so undoOrdering.ts records which one changed last and editablePage/workspaceRoot route Ctrl+Z accordingly. A picture cell is usually smaller than the 63x52 image buttons, so those are hidden inside a cell and the user clicks the picture itself. Video cells, the Canvas palette entry, the spreadsheet round trip and the end-to-end suite follow in later PRs.
CKEditor copies a box's content when CKEDITOR.inline() creates the instance and writes that copy back into the box when its asynchronous startup finishes. The box is contenteditable the whole time, so anything typed in between is silently overwritten by the copy. A table cell hits this every time. The library builds the cell empty, Bloom attaches an editor to it, and the cell can be typed in at once; instrumenting a run measured the startup taking between half a second and 1.3 seconds, and the write at the end of it destroying a word that had been typed and verified in between. Two of the table end-to-end tests failed on this, in different places from run to run, according to where the timing landed. So remember what the box holds just before the startup write - our "loaded" handler runs before it - and, if the write changed the text, put the content back and hand the editor the same content, so what was typed survives and the editor agrees with the DOM. The comparison is on the text rather than the markup, because the startup write also tidies an empty paragraph (<p></p> becomes <p><br /></p>) when nothing was typed, and that is not a loss worth undoing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
First of six stacked PRs splitting #8315 (107 files) into pieces a human can review. Merge in order. #8315 stays open, untouched, as the reference for what the finished tree should look like.
The stack:
BL-16818-table-on-canvas— a table as a canvas elementBL-16818-table-video-cells— video as a cell content typeBL-16818-e2e-hooks— the production hooks the e2e suite needsBL-16818-e2e-tests— the table e2e suiteBL-16818-spreadsheet-tables— the spreadsheet round tripThe Alphabet Book template page left this work entirely; it is BL-16823, branched from this PR.
What to try
Turn on Tables in the collection's Advanced settings, then choose Table in an origami section's type chooser and edit it. Type in cells, add rows and columns, put a picture in a cell, undo across both the table's history and CKEditor's.
What is here
The dependency.
bloom-tableis not on npm and is not going to be; it ships as a committed-disttag on GitHub, the same mechanismbloom-ai-image-toolsuses.package.jsonpinsgithub:BloomBooks/bloom-table#dist-v1.0.0, andbasePage.lessinlines the library's stylesheet fromnode_modules. Vitest needstest.server.deps.inlinefor it: the library's ESM build imports MUI by subpath, MUI 5 ships noexportsmap, and Node's loader rejects those as directory imports. CI green here is load-bearing — it is the first proof this dependency resolves anywhere but a developer's machine.The gate. Pro tier, behind a
tablesexperiment.FeatureRegistrygainsFeatureName.TablewithPreventPublishingInOriginalBooks;ExperimentalFeaturesgainskTables, surfaced by the Advanced tab checkbox.The table.
tableEditing.tsattaches a table, wires each cell's translation group for editing — including cells the library builds after page load, and nested tables — and tears the listeners down again. Undo has two stacks now, the table's and CKEditor's, soundoOrdering.tsrecords which one changed last andeditablePage/workspaceRootroute Ctrl+Z accordingly. A picture cell is usually smaller than the 63×52 image buttons, so those are hidden inside a cell and the user clicks the picture itself.Video cells, the Canvas palette entry, the spreadsheet round trip and the end-to-end suite follow in later PRs.
Checks
pnpm lint,pnpm typecheck,pnpm test(800 passed),build/agent-vite.sh, andFeatureStatusTestsall green on this branch alone.[Claude Opus 5 following a prompt from Hatton]
Devin review
This change is