Bloom Tables (BL-16818) - #8315
Conversation
Integrate the bloom-table library into Bloom as a new "Table" page section
type plus a toolbox tool, powered by the library's DOM-persistence model
(div.bloom-table.grid + div.cell with data-* attributes; all state lives in
the HTML, matching Bloom's save pipeline).
Front-end:
- tableEditing.ts (new): registers Bloom cell content types (translationGroup
default + image), attaches/detaches the library on every .bloom-table, and
wires new cell content (BloomField.ManageField / SetupImagesInContainer).
- origami.ts: "Table" layout option that inserts a .bloom-table.grid and
attaches it.
- bloomEditing.ts: SetupTableEditing on page load; save-cleanup removes table
editing artifacts and tears down the library before persisting HTML.
- toolbox/table/ (new): TableTool hosts the library's TableMenu in the toolbox,
injecting a page-frame TableApi (via editablePageBundle.getTableApi) so the
panel's structural ops run in the realm where the tables are attached.
tableToolPageBridge.ts fetches that api type-only. Registered in
toolboxBootstrap, Settings.pug ("More..."), and toolIds (kTableToolId).
- editablePage.ts: expose getTableApi (and tableCanUndo/tableUndo/getTableApi
on the cross-frame IPageFrameExports, fixing a latent type gap).
- workspaceRoot.ts: route Ctrl+Z to table undo first; bundle plumbing.
- basePage.less: structural grid CSS so tables render at edit time and in
published books (bloom-player/PDF run no JS).
C#:
- ExperimentalFeatures.cs / FeatureRegistry.cs: "tables" token + Table feature
(detected by .bloom-table on the page).
Localization:
- EditTab.CustomPage.Table and EditTab.Toolbox.TableTool (en, dynamic).
bloom-table is consumed via yarn link for now (see BloomBrowserUI/package.json
note); not yet a published dependency. Runtime-verified end to end: add table,
type multilingual (en/fr) text, structural edits from the toolbox panel, undo,
and a clean save/reload round-trip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- tableTool.tsx imports "bloom-table/table-menu.css" so the panel's Tailwind-style utility classes have CSS in Bloom (which ships no Tailwind); without it the panel collapsed to a vertical stack. - ToolboxRoot.tsx + tableTool.svg: toolbar icon for the Table tool. - bloom-automation SKILL.md: document CDP patterns for driving Bloom's editable page (frame access, force-click past overlays, save via tab switch, undo via workspaceBundle.handleUndo, toolbox tool activation, console-error capture, restart after relinking a front-end dep). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bloom-side work for the Table toolbox tool: - vite.config.mts: exclude the yarn-linked bloom-table from optimizeDeps. Vite pre-bundled it and ignores node_modules for change detection, so rebuilding its dist never reached the running toolbox/page. Excluding it serves the linked dist live, so a `vp pack` shows up on the next reload. - tableTool.css: host-specific layout for the panel in Bloom's narrow toolbox column (fill width, no chrome, smaller buttons). Wrapping + wrapped-row gap now come from the library's table-menu.css, so they are no longer duplicated here. - basePage.less: stop duplicating bloom-table's structural/read-time CSS; inline-@import it from the library so it stays in sync and still ships to no-JS reader/publish views. - tableEditing.ts: minor wiring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the one conflict, in SettingsToolControls.tsx: master replaced the hand-written list of <ToolboxCheckbox> elements with a kToolDefs array rendered by a map (and added the shouldCheck/checkedState wiring), while this branch had added a Table Tool checkbox to the old hand-written list. Kept master's data-driven rendering and moved the Table Tool in as a kToolDefs entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bloom-table has grown its own in-page editing affordances: proximity-gated pills for the table, row and column, edge insert buttons, and right-click menus, all built with plain DOM in the page frame. That makes the toolbox panel redundant, and the toolbox was always the awkward half of the integration: it lives in a separate iframe, so every operation had to be routed back into the page frame through an injected TableApi. So this removes the whole toolbox side of the feature: - delete the TableTool, its page bridge, icon and stylesheet - unregister it from toolboxBootstrap, toolIds, ToolboxRoot and the toolbox settings checkbox list - drop getTableApi() from the page frame's exports, which existed only to give the toolbox-hosted panel a page-frame realm to run in - drop the EditTab.Toolbox.TableTool string (never translated) Undo stays wired: tableCanUndo()/tableUndo() are still exported from the page frame and still checked first in workspaceRoot's undo chain. Also brings the cell content types up to date with the library. Bloom used to register a fifth type, "translationGroup", which left the library's own bare-contenteditable "text" type in the cell menu alongside it. Bloom now replaces the library's "text" and "image" types in place, keeping their names and icons, and unregisters "video", whose template is a plain <video> pointing at a sample on the web that Bloom cannot honour. The library gains unregisterCellContentType for that. Finally, origami no longer adds the library's old "table" class to a new table; the library renamed its structural classes to bloom-table/bloom-cell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…able rows/columns A table dragged onto a canvas is now built inside finishAddingCanvasElement's doAfterElementCreated callback. Building it afterwards was too late: that function ends with refreshCanvasElementEditing, which finds the empty bloom-table div and attaches a table of its own whose rows hug their content, so the later call added a second two rows and columns (eight cells, with a thin dashed band at one edge) and could not make the sizes grow. The element also gets bloom-noAutoHeight, since its cells hold translationGroups that would otherwise fight the row heights. The text-cell template now carries its own bloom-editable, so a new cell can be typed in, and shows its format gear and language name, without waiting for the next page load to have C# add one. A canvas element holding a table is many text boxes, not one, so: - StyleEditor.AdjustFormatButton no longer moves the gear out to the left of the whole element when the box is inside a .bloom-cell; the gear stays in the cell. - editMode.less lets the cells' language names through while the page is hovered, beating the rule that hides a canvas element's names unless the bloom-canvas is. - CanvasElementContextControls omits the language name from the context bar, where it would have named only the first cell. Row and column splitters now win over dragging the element. Bloom listens for mousedown on the .bloom-canvas in the capture phase, and the Comical canvas is painted over the table, so the press never reaches the table on its own; instead CanvasElementPointerInteractions hands it to bloom-table's new point-based beginResizeAtPoint, and shows the resize cursor via resizeEdgeAtPoint for the same reason. The control frame's handles are still checked first, so the element's own edges keep resizing the element. Also included: a tableEditing unit test covering the two-by-two shape and the editable per cell, a table item for the canvas palette, and a papercut about a rebuilt linked bloom-table not reaching the running Bloom until the Vite dev server is fully restarted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cells The library's public API, class names and markup are unchanged since Bloom's integration point (14 commits; src/index.tsx is byte-identical, one CSS property changed), so nothing failed to compile. The gaps were behavioural and only showed up in the running app. Cells the library builds after the page has loaded now get Bloom's editing wiring. A new row or column, every cell of a nested table, a cell whose content type changed, and the whole table after an undo all arrive with no contenteditable and no CKEditor, so the user cannot type in them. tableEditing now listens for the library's own tableHistoryUpdated (dispatched on the document at the end of every history-wrapped operation) as well as tableCellContentChanged, and runs an idempotent pass that sets contenteditable before attachToCkEditor (CKEditor's inline mode reads the element's contenteditable state at attach time), then ManageField, long-press, the language tag, and SetupImagesInContainer. Teardown finds tables by class rather than by our data-table-attached marker, because the library attaches nested tables itself. A picture cell showed no placeholder, for two unrelated reasons. placeHolderImages.less hid it. The rule that suppresses the placeholder when a bloom-canvas holds canvas elements other than its background image was written with descendant combinators, so the page's own canvas elements satisfied its :has() and the rule reached into every cell's nested bloom-canvas. The data-tool-id="canvas" easel rule leaked the same way. Both now use child combinators, which is what "belongs to this canvas" means; the one case where a canvas element is not a child of its bloom-canvas is while the Image Description tool has them wrapped, and that case hides the placeholder anyway. The picture was also fitted to the wrong box. The table lays itself out with JavaScript, so on page load a cell's bloom-canvas can still have no height when Bloom fits the picture; the geometry written then becomes the baseline that later resizes scale, and the offsets grew with each pass until the picture sat almost entirely outside the cell. Fitting now bails out on a canvas with no area, CanvasElementManager gained refitBackgroundImage for working the geometry out afresh, and the table's refit calls it for each cell that has a real size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bloom's capture-phase mousedown handler claimed every right-press on a canvas element and rendered the canvas element's controls (Duplicate, Delete). Since mousedown precedes contextmenu, bloom-table's document level contextmenu handler never ran, so a right-click in a cell showed the wrong menu -- on picture cells and text cells alike. A right-press that lands in a table now returns early and leaves the contextmenu that follows to the library. event.target cannot answer "is this a cell", because the Comical canvas covers the table, so the test is structural: the bloom-canvas is inside a .bloom-cell, or the canvas element under the pointer holds a .bloom-table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library offers a video cell whose content is a bare HTML5 <video>, which none of Bloom's video tooling knows about, so Bloom removed the type from the cell menu. Register Bloom's own markup for it instead: the bloom-videoContainer that origami's Video link creates, which the Sign Language tool records into, the hover controls play, and the publishing code collects videos from. The library's name and icon are kept. A new video cell gets the wiring Bloom's page-load pass would have given it, and asks the server for the book's copy of the placeholder graphic, as origami does. Containers already wired are remembered, because we run over a whole table again after every table operation and the click that opens the Sign Language tool has no guard of its own. Two things in the video code assumed a video container is alone in its canvas element, which is no longer true when the canvas element holds a table: the Sign Language tool took the first container it found there rather than the selected one, so a second video cell could never be recorded into, and a click on a video in a cell was suppressed while the Canvas or Game tool was showing, which left no way to reach the tool that records into it. A video cell is not one of the canvas elements those tools arrange, even though the table around it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # PAPERCUTS.md # src/BloomBrowserUI/collection/AdvancedSettingsPanel.tsx # src/BloomExe/Collection/CollectionSettingsDialog.cs # src/BloomExe/ExperimentalFeatures.cs # src/BloomExe/web/controllers/CollectionSettingsApi.cs
…helpers E2E (src/BloomE2E): - tables-core.spec.ts (10 tests, 4 fixme) and tables-extended.spec.ts (16 tests, 5 fixme), each step wrapped in the new `step` fixture. Both pass three consecutive runs from a clean state. - New helpers: tables, canvasElements, toolbox, keys, geometry, origami, pageSize, videos, caption; bookHtml/bookMaking/images/publish/workspace extended for tables. fixtures/videos/short.mp4 for video cells. - Slow-step reporter (reporters/slowSteps.ts) lists named steps over BLOOM_E2E_SLOW_STEP_MS (default 3000) at the end of every run. - The fixme steps record suspected product bugs; the first-click-swallowed finding is BL-16815 and the 54-cell page load is BL-16816. Step caption (src/BloomBrowserUI/app/e2eCaption): a three-line strip at the bottom of the workspace shell, rendered only when common/instanceInfo reports runningE2eTests, showing the test clock, the current step with a live timer, and finished steps with their measured durations. Driven by window.bloomE2eCaption from the `step` fixture. Product: - Tables gated behind the `tables` experimental feature and the Table tier feature (tableFeature.ts, CanvasToolControls, CanvasElementItem, control registry, TextContextMenu, keyboard provider tests). - Spreadsheet export/import of tables via the hidden [details] column (SpreadsheetTables.cs, SpreadsheetTableTests.cs). - ExperimentalFeatures honours an e2e-supplied feature list; E2eTestingApi, SignLanguageApi and CommonApi gain the routes the specs need. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
tables-gating.spec.ts: three tests, each restarting Bloom into the state it
needs. Below Pro a derivative's table takes text and picture replacement but
no restructuring chrome; below Pro the original book cannot be published;
with the tables experiment off the table is frozen and nothing offers a new
one. Three product mismatches are test.fixme with the observed behaviour:
a video-only table is inferred as a video element; right-clicking a cell
below Pro opens no menu at all; there is no route to copy a table element
below Pro so the paste dialog cannot be exercised.
Fixtures: launchBloom gains IRelaunchChanges so a restart can replace
startup-only options (subscription, experimental features); bloomTest and
restartWithCollectionSettings pass it through. ExperimentalFeatures.cs now
treats the e2e environment variable as the whole answer under --e2e ("none"
means none), since the saved setting shares user.config with the developer's
own Bloom. New helper requiresSubscription.ts; additions to origami,
canvasElements, tables, publish and addPageDialog helpers; test ids on the
subscription dialog and the publishing-blocked notice.
Collection readiness limit raised to four minutes after two launch timeouts
while another worktree's suite was starting its own Bloom (AUTOMATION-DEBT,
PAPERCUTS). Menu closing falls back to clicking the backdrop when Escape
does not close a MUI menu.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n menu A table on a Canvas page lives inside a canvas element, and the text context menu's paragraph finder (noIndent.ts findParagraphForTextContextMenu) rejected anything inside one, so a frozen table's text cell got no menu at all. A paragraph inside a .bloom-cell is now let through before that rejection. A press on a picture cell in a frozen table (CanvasElementPointerInteractions isPressInsideTable) now steps aside so the picture opens its ordinary menu. Video cells still get no menu at any tier: a video in a cell is a bare video container and Bloom's video commands hang off a canvas element's menu. Tests: noIndentSpec covers a cell paragraph inside a canvas element; tableEditing.gating.test.ts asserts the right-click reaches a document-level listener and the paragraph is one the text menu claims. tables-gating.spec.ts turns the right-click fixme into two live cases (text cell gets the text menu, picture cell gets the picture menu, Cell menu stays shut) and drops the paste-dialog fixme: copying a table is itself Pro, so that case is unreachable by design. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…BL-16818) A gated feature is not tested until the books it makes have been exercised by someone without the subscription: localizing a derivative, exactly what is frozen, the original book's publish block, and the experimental switch off. The expected behaviour is a product decision to get from the developer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Too many files changed for review (104 files, 100 file limit). Bypass the limit by tagging |
| // First see if origami is active and knows about something we can undo. | ||
| // (Origami undo works only while the origami tool is active.) | ||
| const contentWindow = getEditablePageBundleExports(); | ||
| // Check table (bloom-table) undo first. Structural grid ops leave no |
There was a problem hiding this comment.
[Devin] Investigate: Structural undo drops text history
Undoing a table structure change (adding a row, say) rebuilds the cell editors and with them their typing history. After typing in a cell and then adding a row, a second Undo cannot take back the typing.
|
[Devin] Investigate: PR targets the wrong branch ( The repository instructions say new work should target Decided not to act on this: the developer chose master as the base for this PR explicitly. The Version6.5 note in AGENTS.md is a temporary transition rule and does not apply to this feature branch. [Claude Fable 5.1 from Hatton's machine during preflight] |
…ests A duplicated table now takes commands of its own. Duplicating a canvas element cloned the table's markup including the data-table-attached marker Bloom writes on a table it has wired up, so attachSingleTable skipped the copy and none of its row, column or cell commands reached the table library. The clone cleanup now drops that marker. Unit test added; the tables-core E2E test that covered this is no longer fixme. A table holding a video is now classified as a table. inferCanvasElementType tested for a video container before it tested for a table, so such a table got a video's toolbar, with Duplicate ungated below Pro. The table test now comes first. Unit test added; the tables-gating E2E test is no longer fixme and now opens the derivative's own video page from the Edit tab. The "shows both tables' pictures in a BloomPUB preview" test stays fixme: wiring the copy up did not cure the missing picture. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…readable A [table] row whose [details] cell had been blanked or edited into invalid JSON was parsed as null, and BuildTableFromDetails happily built an empty table from that, replacing the book's real table with one that had no cells. Now the importer warns and skips the table's rows instead, as it already did for a spreadsheet with no [details] column at all. Found by Devin's review of the PR. Covered by a parameterised unit test for the empty and the malformed case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Spreadsheet import: when a [table] row's details cell is unreadable, the book's table is now used up before the rows are skipped, so the next [table] row still lands on its own table rather than one table early. (Devin, on the previous commit's guard.) Test with two pages, one table each, the first row spoiled and the second edited. - The cell-content-changed listener now goes on the document, like the history listener, instead of on whatever subtree SetupElements was given. A canvas element being added re-ran SetupTableEditing on its bloom-canvas, and a cell's event then reached both listeners and wired the cell twice. Teardown removes it from the document too. - The table feature status is fetched once per page load; each added canvas element used to fetch it again. - Only the main mouse button can start a row or column resize, so a right-click near a boundary still opens a menu. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
When a [table] row's details cannot be read and the book has no table left for it, the importer used to advance anyway, which cloned the last content page, and then skipped the row, leaving an unchanged duplicate page behind. Only a row Bloom can actually use may be the reason a page is added. (Devin, on the previous commit.) Test: two table rows, one-page book, second row spoiled; the page count stays at one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| // a shape nothing in the spreadsheet asked for), so if there is none to be had | ||
| // we say so and skip rather than advancing off the end of the book and adding a | ||
| // page that still has no table for us. | ||
| // A row whose details cannot be read (handled below) may use up a table the |
There was a problem hiding this comment.
[Devin] Bug: Unreadable table rows add pages
After 2dee5e1, a table row whose details cell could not be read was moved onto the book's table before being skipped. When the book had no table left for it, that move added a page (a copy of the last content page) and then left it untouched, so importing a spreadsheet with a spoiled table row could leave a duplicate page in the book.
There was a problem hiding this comment.
[Claude Fable 5.1 from Hatton's machine during preflight] Confirmed and fixed in d012728: a row Bloom cannot read may still use up a table the book already has, but it is never the reason a page gets added. Unit test: a two-table spreadsheet into a one-table book, with the second row spoiled, leaves the page count alone.
There was a problem hiding this comment.
[Claude Fable 5.1 from Hatton's machine during preflight] Devin's follow-up on d012728: one case remains. A spoiled table row that also names a page type different from the page it should land on can still make import insert a template page of that type before the row is skipped. That is a spreadsheet damaged by hand in two ways at once, and the import warns about it, so whether to restructure the page-advancing code for it is in the preflight report as a decision. Thread left open until that decision comes back.
|
[Claude Fable 5.1 from Hatton's machine during preflight] Consulted Devin on 2026-09-05 up to commit d012728 (five passes, one per push). Devin raised seven bugs and six things to investigate; each has its own review thread with what we did.
No informational flags. Greptile declined the PR (over its file cap), CodeRabbit is not active here, and the pr-automation check passed on every push. |
…s picture A table's structural operations go through the bloom-table library's history and typing inside a cell goes through CKEditor's. Neither stack knows about the other, and the library keeps an entry until it is undone, so "add a row, then type in a cell" leaves both non-empty at once. Asking the table first took the row back out while the typing, which came later, was what the person meant. undoOrdering.ts records the order the two stacks were last written in, on a shared sequence number, and decides from that. A bare counter rather than a clock, because two changes in the same millisecond are ordinary and a tie has no right answer. The decision is a pure function so it can be tested on its own; editablePage exposes tableShouldHandleUndo() cross-frame alongside the plain tableCanUndo(). The library side of this is bloom-table's in-place undo of an insertion: undo used to restore a snapshot of the whole table's innerHTML, which replaced every cell and so discarded the CKEditor instances on the cells that were not part of the insertion, putting the typing they held permanently out of Undo's reach. Also refit the picture in a picture cell when its cell changes size, which the library now does as a container narrows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The library was reachable only through `pnpm link` from a sibling checkout, so nothing here built on any other machine: nineteen files import bloom-table and basePage.less inlines its CSS out of node_modules, but no dependency declared it. It is not going on npm, so it ships the way bloom-ai-image-tools does, as a tag whose commit carries the built dist. Installing it properly surfaced a failure the symlink had been hiding: every test file that imports bloom-table failed to load with "Directory import '@mui/material/Divider' is not supported". Its ESM build imports MUI by subpath, MUI 5 ships no "exports" map, and Vitest hands installed dependencies to Node's ESM loader, which rejects directory imports. A symlinked package never took that path. test.server.deps.inline puts bloom-table through Vite's resolver instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem
Bloom has no way to lay content out in a grid. Authors who want an alphabet chart, a vocabulary list with pictures, or a simple timetable have had to fake it with positioned text boxes, which do not stay aligned and cannot be translated as one unit. Tables are a Pro-tier feature, and the people who reuse Pro books, above all to translate them, often have no subscription, so a table must stay translatable and publishable in their hands while they are kept from building new ones.
What the PR does
bloom-tablelibrary supplies; whole tables are canvas elements and duplicate, move and delete as such.tablesexperimental setting. Below Pro, or with the setting off, an existing table is frozen: its text and pictures stay editable and its cells get the ordinary text and picture menus, but nothing can restructure it, the palette icon and Add Page entries are absent, and Change Layout's Table link shows the subscription dialog. An original book with a table cannot be published below Pro; a derivative publishes untouched, the same rule canvas elements follow.[table]row and one[table cell]row per cell with the table's attributes verbatim in the hidden[details]column; import rebuilds the table from them, and leaves the book's table alone when a sheet cannot describe one (C# unit tests included).fixmefor suspected product bugs), a step caption at the bottom of the Bloom window during--e2eruns showing each step and its timing, a slow-step reporter, and helpers for canvas elements, keyboard, video import and relaunching Bloom under a different tier.Not yet:
bloom-tableis a localpnpm link, not an npm dependency, so CI cannot build this branch until it is published. Spreadsheet round trip as an E2E test waits on BL-16813.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16818
Devin review
This change is