Skip to content

Org chart user-chosen page breaks (#4245)

A user clicks a person in the live preview and picks “Put this team on its own page” (or “Remove page break” if already marked). That person’s subtree is then extracted onto its own page/section wherever the chart is rendered.

Architecture

  1. Split step — packages/org-chart-shared/src/page-breaks.ts’s splitTreeForPageBreaks removes each marked subtree from the main tree and produces a stub (name/title/headcount) in its place, plus the extracted sections (including nested breaks within a section).
  2. Template wrapper — workers/api/src/templates/index.ts’s getTemplate(id).render() applies this generically via renderWithPageBreaks when input.options.pageBreaks: string[] is set: it renders the main tree (with stubs), renders each section with the SAME template, demotes the section’s own <h1> to <h2>, adds a back link, and concatenates everything into one document. Each template only needed one small, mechanical change — its “render one person’s box” function checks person._pageBreakStub (see top-down-tree.ts’s renderNode for the reference patch). templateSupportsPageBreaks gates this off for interactive-collapsible and the category: 'interactive' templates (radial, civic-accessible) — none of the three have a “page” concept.
  3. Paginated PDF — services/org-chart-pdf/index.ts’s buildPaginatedPages takes the same pageBreaks list and forces a page cut per marked subtree (auto-split for overflow still runs inside each piece).
  4. Breaks API — workers/api/src/routes/org-chart/breaks.ts, mounted at /api/orgcharts/:id/breaks. Breaks are saved per output/view (issue decision): viewId given → org_chart_views.page_breaks; omitted → org_charts.page_breaks (the chart’s own default view).
  5. Preview UI (this change) — a plain click, or Enter/Space outside the drag-and-drop layer’s keyboard move-mode, on a person node in the editable preview iframe opens a small menu offering whichever single action applies (detected from whether the node already renders as a stub). The iframe has no auth, so it posts orgchart:pagebreak {action, personId} to the host (apps/org-chart/src/app/preview/page.tsx), which makes the actual API call, optimistically updates local state, and re-renders every cached preview from the new break set (regeneratePreviews({ pageBreaks })). See workers/api/src/utils/org-chart-html.ts’s PREVIEW_INTERACTIONS_JS/ PREVIEW_INTERACTIONS_CSS and apps/org-chart/src/lib/reparent-protocol.ts.

Radial and civic-accessible

Radial and civic-accessible are fundamentally different from the other templates: they’re infinite, zoomable D3/SVG canvases built from a CSV fed through a temp /share?token=...&embed=1 iframe, not from PREVIEW_INTERACTIONS_JS — so the HTML templates’ split/stub/section machinery doesn’t apply to them directly. An earlier revision of this doc said marking here persists but has no visual effect (“no page concept in an infinite canvas”). That undersold the requirement — the issue calls for the SAME effect (stub + extracted section + back link) in every layout except interactive-collapsible. It’s now implemented:

  • Split, client-side. apps/org-chart/src/components/templates/shared/ pageBreakSplit.ts’s buildPageBreakPartitions is the client equivalent of renderWithPageBreaks: it flattens the already-built OrgChartNode tree to {id, managerId} pairs, runs the SAME splitTreeForPageBreaks from @org-chart/shared the backend uses, then REBUILDS a fresh nested tree per partition (main chart, plus one per extracted section) — the split result is only ever a reordered/filtered id list, never ready-made nested nodes, so each partition’s children are re-linked from its own flat edges only (never reusing the original un-split subtree).
  • One RadialOrgChart/CivicTreeChart instance per partition. org-chart-radial/index.tsx and org-chart-civic/index.tsx render the main chart from partitions[0], then partitions.slice(1) as <section id={partition.id}> blocks below it — each with an <h2> heading (“{name}’s team”, matching the HTML templates’ wording), a back link (to the parent section or #main-content, same convention), its own independent re-centering state, and breakBefore: 'page' for print.
  • Stub rendering in the SVG itself. Both RadialOrgChart.tsx and CivicTreeChart.tsx check node._pageBreakStub in their D3 node-render code and draw a HOLLOW, DASHED circle/rect (shape, not color — WCAG 1.4.1) with the headcount as visible text, plus a genuine <a href="#sectionId"> (via foreignObject + a real HTML anchor — SVG’s own <a> has inconsistent support/testability, a plain anchor does not) that jumps to the section (pageBreakSplit.ts’s jumpToAnchor: scrolls the target into view and moves DOM focus to it, the standard same-document-link technique, since a section’s <h2> isn’t natively focusable).
  • Fixed SVG id collisions, fixed. RadialOrgChart now takes an instanceId prop and prefixes every fixed id (gradients, filters, the node tooltip) with it. url(#id) references resolve document-wide in SVG, not per-ancestor-<svg>, so two unscoped instances on one page (main + N sections) would otherwise silently steal each other’s gradients/filters. CivicTreeChart has no such fixed ids (verified, not assumed) — nothing to scope there.
  • Keyboard/screen-reader operability is split across two surfaces, by necessity:
    • Radial’s SVG already has real roving-tabindex/role="button"/ aria-label mechanics on every node (used for re-centering). A stub node keeps those — activate() branches on _pageBreakStub to call jumpToAnchor instead of re-centering, so Enter on the roving-focused node does the same thing the nested anchor’s click does. The anchor itself is tabindex="-1" (the surrounding node is the one real tab stop, consistent with every other node) but still a real link, so a screen reader’s link rotor finds it independent of tab order.
    • Civic’s SVG is aria-hidden="true" (pre-existing — its nodes are otherwise mouse-only and unlabeled; this change didn’t alter that). A descendant can opt back IN with its own aria-hidden="false", which the stub’s anchor does — a real, additionally keyboard-reachable (tabindex="0") link inside an otherwise-hidden canvas. But the AUTHORITATIVE accessible surface for civic (and radial) remains the shared AccessibleTree component (shared/AccessibleTree.tsx, used by both templates’ list views) — it now renders a stub row’s headcount as a real <a href="#sectionId"> in place of the normal children-count badge, and handleSelect redirects Enter/click on a stub row to jumpToAnchor instead of selection. This is the PRIMARY way a keyboard or screen-reader user reaches a section in civic.
  • Real person ids. Radial/civic’s CSV pipeline previously had no guaranteed link back to the chart’s real org_chart_people.id (nodes get a synthesized per-row id when the CSV has none — see org-chart-radial/utils.ts’s idForRow, which already prefers a provided id). buildShareUrl() in preview/page.tsx now emits a real id column, so both the split step and the mark/unmark control always use an id the breaks API recognizes.
  • Marking is still via PageBreakControl, not the node’s own click/ Enter. In both templates, click/Enter on a node is already bound to “recenter/select this person” — overloading it the way the HTML layer does would create an ambiguous double-action. Selecting a person reveals a PageBreakControl (shared/PageBreakControl.tsx) button in the “currently selected person” panel both templates already show — unchanged from the original implementation; this round of work only changed what a mark’s EFFECT looks like, not how a mark is requested.

Known, scope-limited gap: the page-level search box and breadcrumb trail operate on the MAIN partition’s tree only (same as the HTML templates, which don’t carry search into their sections either) — searching for someone whose subtree was extracted into a section won’t find them; the user reaches that person by scrolling to (or clicking “See section” into) the section itself.

Auth path (superseded — see “Review fixes” below): /share?embed=1 runs on the SAME origin/session as the rest of the app when loaded inside the preview page, but now routes mark/unmark through postMessage to the host instead of calling the breaks API directly when a real host window is present — gated on embed && user so a genuinely public share link (no session) never offers editing.

Review fixes (#4245 follow-up, 2026-10)

templateId is now required server-side whenever viewId is omitted. The no-view default-scope breaks are stored per template (org_charts.page_breaks is { "<templateId>": ["<personId>", ...] }, not a single shared array) — marking a break while viewing one static layout no longer silently applies to a different layout rendered without a view. GET /api/orgcharts/:id/breaks?viewId=&templateId=, POST body { personId, viewId?, templateId? }, DELETE .../breaks/:personId?viewId=&templateId=. apps/org-chart/src/lib/api.ts’s getPageBreaks/markPageBreak/ unmarkPageBreak take an added templateId param; every call site in preview/page.tsx passes the currently selected template, and share/page.tsx passes shareData.templateType (radial or civic-accessible). Omit templateId whenever viewId is set (the view already pins one template).

commitPageBreak (preview/page.tsx) now splits the save and the re-render into two tries. A failed breaks-API call reverts the optimistic pageBreaks state and shows the existing assertive error toast. A failed regenerate (the save itself succeeded) leaves pageBreaks alone and shows a distinct message (preview.pageBreak.regenerateFailedToast) — reverting here would lie about what’s actually saved. regeneratePreviews() takes a new quiet option for this path: it skips clearing the currently-displayed previews and skips the page-level fatal error state, so a refresh hiccup doesn’t blank the whole preview pane behind an unrelated “Preview Unavailable” screen.

Per-person in-flight guard, not a single global lock. pageBreakBusyRef is now a Set<string> of person ids currently mid-request. A second mark/unmark for the SAME person while one is in flight is dropped; a DIFFERENT person’s action proceeds independently. PageBreakControl.tsx takes a busy prop (used by share/page.tsx for radial/civic) that marks the trigger aria-disabled with an aria-describedby reason — never native disabled, which some screen readers skip without explanation. For the static HTML templates’ in-iframe menu, the host posts orgchart:pagebreak-busy {personId, busy} acks (see reparent-protocol.ts), and PREVIEW_INTERACTIONS_JS (workers/api/src/utils/org-chart-html.ts) now consumes them: the busy person’s node gets aria-disabled="true", opening their menu announces “a page-break change is already in progress” instead of opening it, and an ALREADY-OPEN menu for a person who becomes busy mid-interaction has its one action item disabled in place (aria-disabled + the native disabled property) rather than letting a stale click post a second request. See workers/api/src/__tests__/templates/preview-interactions-behavior.test.ts’s “page-break in-flight busy state” suite for the behavioral proof (real jsdom execution of the injected script, not just a string-marker check).

The initial getPageBreaks load can fail, and now says so. Both preview/page.tsx and share/page.tsx track pageBreaksLoaded / pageBreaksLoadError; a failed load shows a visible, dismissal-free banner with a Retry button and withholds the editing affordance (the static template’s menu simply has nothing to open against; share/page.tsx omits pageBreakEditor entirely) until a retry succeeds — editing against an unknown/possibly-wrong break set was silently treated as “no breaks exist” before this fix.

The embedded /share editor routes through the host, not the API, when a real host window is present. share/page.tsx computes routeThroughHost = embed && window.parent !== window (the embed param alone doesn’t guarantee an actual parent frame — e.g. a signed-in user can open ?embed=1 directly). When true, mark/unmark posts the SAME orgchart:pagebreak {action, personId} shape the static templates’ injected menu already sends (pageBreakMessage() in reparent-protocol.ts), and waits for the host’s orgchart:pagebreak-result {personId, success, message?} reply (parseHostPageBreakMessage()) to settle its own optimistic state and busy flag. This keeps the host’s pageBreaks state in sync so its NEXT unrelated regeneratePreviews() call (e.g. switching static templates) never re-renders with a stale list and undoes an edit made inside the embedded view. The host’s preview/page.tsx message listener now validates the message source against EITHER the static-template iframe or the embedded /share iframe (interactiveIframeRef) — previously only the former was tracked, so page-break messages from the embedded editor were silently ignored. When there’s no real host window, share/page.tsx falls back to calling the breaks API directly (unchanged standalone behavior).

Visual QA round (#4245 follow-up, 2026-10, “v2”)

Real-path screenshots (generate-previews + previews/:templateId, and buildPaginatedOrgChartHtml for the paginated PDF — never a template called directly) surfaced three rendering bugs and one contrast bug, all fixed on this branch:

  1. The main chart area rendered empty whenever sections existed. ZOOM_CONTROLS_CSS (every template) makes body a height:100dvh flex column with a single .chart-scroll set to flex:1 1 auto — an “app shell” that assumes exactly one scroll area per document. A page-break document stacks the main chart + one .chart-scroll per section as flex siblings of that same body, so they compete for one fixed 100dvh and the main chart could be squeezed to near-zero height even though its content geometry was computed correctly. Fixed in PAGE_BREAK_CSS (workers/api/src/templates/index.ts): body { height: auto; display: block } / .chart-scroll { flex: none; height: auto; overflow: visible } overrides restore natural document flow. Regression-guarded by workers/api/src/__tests__/templates/page-breaks-layout.chromium.test.ts, which renders the real template output in a headless Chromium and asserts the root person’s box and the stub’s box both have non-zero, in-viewport bounding rects.
  2. Back links and stubs rendered in the browser-default link blue (#0000EE), not the template’s own tokens. .page-break-back-link a and .node-pagebreak-link had no color set. Every template already defines --color-primary/--color-primary-dark (theme-aware, redefined under the dark/alt theme), so PAGE_BREAK_CSS now sets color: var(--color-primary-dark, var(--color-primary, #1e40af)) on both.
  3. The stub’s “N people / See section” text floated centered, detached from the person’s row, in list-style templates (indented-list, etc.). .node-pagebreak-stub used display:flex; flex-direction:column; align-items:center, which centers content across the stub’s full row width regardless of the template’s own text alignment. Changed to display:block so it inherits the ancestor’s text-align — centered in card templates that already center their box content, left-aligned (inline under the name) in list templates.
  4. (Proactively, found during the all-template sweep, not explicitly requested) executive-profile’s demoted section heading was illegible. .chart-header h1 { color: var(--color-on-primary-dark); background: var(--color-primary-dark) } is a tag selector that stops matching the instant demoteHeadingAndAddBackLink renames <h1>→<h2>, stranding default dark text on the still-dark banner. Fixed by wrapping the demoted heading + back link in an explicit forced-contrast <div style="background:var(--color-bg,#fff);color:var(--color-text,#0E1116)"> — “contrast overrides every other rule” applies even to a template’s own banner styling, accepted as a defensible tradeoff over pixel-perfect banner integration.
  5. Radial’s dark/blue themes: the hierarchy panel’s “N · See section” link failed contrast. AccessibleOrgChart.tsx passed theme.cardReports as the stub’s text color; several radial themes define cardReports as a translucent rgba (alpha ≤ 0.5) for use as a subtle SVG connector/badge tint, not as solid text color — measured at rgba(212, 175, 55, 0.5) on a rgba(30, 36, 51, 0.7) panel background in “Executive Dark.” Fixed by reusing theme.treeFocusRing instead — the same hue, but a guaranteed solid hex already asserted in themes-contrast.test.ts for the focus ring’s own 3:1 requirement (same reasoning the file already documented for focusRing, just not applied to accent). Verified ~7.4:1 on “Executive Dark” (solid gold rgb(212,175,55) on rgb(30,36,51)). Civic’s AccessibleTreeView was unaffected — CIVIC.accent is already a solid hex (#1F3A5F).

541-person chart through the real path. realisticOrgChart541() (workers/api/src/__tests__/services/org-chart-pdf/test-helpers.ts) with 3 breaks (one nested — a VP and one of that VP’s own directors, plus a second independent VP) renders correctly at scale through both the HTML preview route and buildPaginatedOrgChartHtml: the main chart shows the CEO root plus both VP stubs inline with the rest of the tree (no empty area), each section has its own chart + styled back link, and the nested section (“Director 13’s team”) sits after its parent section with a correct “Back to VP of Engineering’s team” link. Already covered by the committed realisticOrgChart541()-based tests in workers/api/src/__tests__/services/org-chart-pdf/page-breaks.test.ts (auto-split-inside-a-forced-section, and the combined main-auto-split + forced-subtree + nested-mark case).

apps/org-chart’s radial/civic back link said “Back to previous section” for ANY non-root section, regardless of which section it actually returns to — unlike the HTML templates, which already said “Back to {parent name}‘s team”. Fixed by adding PageBreakPartition.backLabel to pageBreakSplit.ts’s buildPageBreakPartitions (computed the same way templates/index.ts computes its backLabel: the parent section’s own heading, looked up by parentSectionId, falling back to “Back to full chart” for a section whose parent is the main chart). Both org-chart-radial/index.tsx and org-chart-civic/index.tsx now render partition.backLabel instead of re-deriving a generic label from backHref. Tested in page-break-split.test.ts (data-level) and page-break-template-wiring.test.ts (mounted-component DOM text, both templates, nested marks).

Review round 4 (#4245 follow-up, 2026-10)

  1. Regex-injection in the demoted-heading back link. out.replace(/<\/h1>/i, \${backLinkHtml}${closeWrapper}`)indemoteHeadingAndAddBackLink(templates/index.ts) was a plain STRING second argument —backLinkHtmlembeds a nested section's real parent name, so a name containing$&/`` $ “/$' would splice arbitrary pre/post-match text into every template’s output. This is the one place the earlier round-2 hardening missed (it only ever gets exercised by a NESTED break — a non-nested section’s back link is the fixed string “Back to full chart”). Fixed to a function replacer; added a dedicated nested-hostile-name test across all 11 templates.
  2. executive-profile’s page-break trigger button/CSS shipped unconditionally. .epcard-pb-trigger (and its CSS) rendered on every card with an expand panel regardless of options.previewInteractions — a misleading, inert control on every finalized saved/shared/printed output, and a <button> nested inside the card’s own role="button" (axe “nested-interactive”). Now gated on previewInteractions in both renderCard and a newly split-out EPCARD_PB_TRIGGER_CSS block. RENDER_VERSION bumped 17→18 (the v17 comment’s claim that this was “entirely gated” was the bug — now corrected and actually true). Added an all-templates test asserting no page-break menu control/CSS/JS marker appears when previewInteractions is unset.
  3. Shares/downloads served HTML frozen at finalize time; PDF/image exports always reflected current breaks. GET /share/:token (and its password-gated/fallback siblings in org-chart-share.ts) read the output’s R2 HTML verbatim — no staleness check at all — while handleImageExport always called regenerateOutputHtml fresh. Fixed by reusing the EXISTING regenerate-on-serve machinery both ways: breaks.ts’s POST/DELETE handlers now call markAffectedOutputsStale, flipping render_version on every org_chart_outputs row in the mutated scope (chart+template for the no-view default scope, or the view, matching the same scope model breaks themselves use) to 0 — see #4285 below, this originally wrote null, which the column’s NOT NULL DEFAULT 0 constraint silently rejected on every call; and a new readShareHtml helper in org-chart-share.ts calls the now-exported regenerateStaleOutputInPlace (previously only the authenticated preview/download routes used it) before serving. page-breaks-loader.ts’s doc comment corrected to describe the frozen-snapshot-vs-live-render distinction. Tested end-to-end: org-chart-share-stale-regen.test.ts mints a finalized output whose HTML has no stub, flips its render_version to 0 (simulating what a break mutation does), then asserts GET /share/:token serves the regenerated HTML with the stub. org-chart-breaks.test.ts asserts the staleness side-effect itself (right scope touched, wrong scope left alone, best-effort on failure).
  4. templateId wasn’t validated against the real template registry — any string was accepted into org_charts.page_breaks’s keys. resolveScopeParams (breaks.ts) now checks against TEMPLATES (templates/index.ts) and 400s for an unrecognized id.
  5. scopeFragmentIds never rewrote a multi-id aria-labelledby/aria-describedby/aria-controls/aria-owns value (a space-separated list per the ARIA spec) — its exact-whole-value regex only ever matched a SINGLE-id value, silently leaving every multi-id reference unscoped. Split into a single-value path (id, for — unchanged) and a new token-by-token rewrite for the four multi-value ARIA attributes. scopeFragmentIds exported for a direct unit test.
  6. A lost optimistic-concurrency race (retry budget exhausted) returned 500. That’s a genuine conflict the client should retry, not a server fault. Now returns 409 with code: "CONFLICT".
  7. Stale 10.1.1.4 reference in the migration’s comment (the Node runtime it named was retired 2026-09-24; the API now runs on the EC2 anchor) — corrected.
  8. The Chromium layout regression test (page-breaks-layout.chromium.test.ts) skipped, rather than failed, when Chromium couldn’t launch — including in CI. A skip that can also happen in CI means a broken/missing browser install silently stops the test from gating anything (exactly the kind of gap that let the original layout bug ship). Now: process.env.CI set + launch failure → the test throws and fails; locally without Chromium installed, it still skips cleanly.
  9. PAGE_BREAK_CSS’s .chart-scroll { overflow: visible } fix for the empty-main-chart bug (round 3) introduced a NEW regression: a wide tree’s .chart-zoom-wrapper could spill out of its own scroll container into normal page flow, scrolling the WHOLE PAGE horizontally instead of just that chart. Fixed to max-width: 100%; overflow-x: auto; overflow-y: visible — keeps the “don’t squeeze main to zero height” fix while keeping horizontal overflow contained to each chart’s own box. Verified with the 541-person top-down-tree render (document.documentElement.scrollWidth == clientWidth at 1400px) and a new dedicated wide-fan-out Chromium test.
  10. POST /generate never loaded saved breaks (unlike every other render path) — not currently called by the frontend (which uses /generate-previews + /finalize), but a one-line fix (options.pageBreaks = await loadPageBreaksForRender(...)) closes the gap for consistency / future callers.

Forced-break page named itself, and its stub overflowed (#4255)

Production smoke testing (2026-10-05, the 80-person “RAS Org Chart” chart) found two bugs specific to applyForcedPageBreaks in the paginated PDF (services/org-chart-pdf/index.ts) — the auto-paginated path never had either:

  1. The forced section’s heading/back link named the section’s OWN root (e.g. “Reports to Melody Bell…” on Melody Bell’s own page) instead of her manager. Each forced section is paginated independently via its own paginate() call on a synthetic single-root tree, so that root looked like an ordinary chart root (no parent) to headingFor — losing the ”— reports to {manager}” suffix — and the back-link fix-up afterward used the section’s own root person instead of looking up who that root actually reports to. Fixed by threading a new PaginateOptions .rootParentRef (the manager’s name/title, looked up via managerId against the full, unsplit people list) that seeds the section’s first page’s parentRef before pagination runs, so headingFor produces the same text the auto path would; the back-link fix-up now looks up the same manager instead of the section root.
  2. The forced stub box was sized from the plain node box (layout.ts’s estimateBoxHeight, via whatever height the marked person’s box happened to get laid out with), not estimateStubHeight’s longer stub-link-text estimate, which overflowed its dashed border in WeasyPrint. Root cause: paginate()’s own cut/fit search already sizes a structurally-cut child’s box from estimateStubHeight (buildStubHeights), but a _pageBreakStub marker left on a person by splitTreeForPageBreaks was invisible to that search — it only ever looked at the ids it decided to cut itself. Fixed by having buildStubHeights (and every other attemptFit/footprint call site that builds its own stub-height map — packSubtreesIntoGroups, the part grid path, pickCutCandidate) recursively scan for _pageBreakStub markers anywhere in the tree and merge their estimateStubHeight into the same map, so the layout reserves the correct height from the start — the SAME code path and metrics a structurally-cut stub uses, so the two can’t diverge again.

Regression-guarded by a parity suite in workers/api/src/__tests__/services/org-chart-pdf/page-breaks.test.ts (“forced vs auto parity for an already-cut person”) that marks a person the auto paginator would already have cut (a VP on realisticOrgChart541()’s CEO overview page, plus a nested director case) and asserts the forced path’s heading, back-link name/target, and stub height/text are identical to what the auto path produces for that same person — plus a blanket check that no stub anywhere is sized shorter than its own estimateStubHeight.

Marking the chart’s own root (e.g. the CEO) for a forced page break is a valid, supported case — applyForcedPageBreaks/managerOf (services/org-chart-pdf/index.ts) correctly produce no back link for that section, by design: the whole point of a back link is “whoever’s page the stub landed on,” and a chart root has no manager, so there’s no page to point back to. This isn’t a bug, just worth calling out explicitly: every OTHER forced section gets a “Back to {manager}‘s team” link, but a root-rooted section’s first page simply has none.

markAffectedOutputsStale wrote render_version: null, which the NOT NULL column rejected (#4285)

Round 4 item 3 above added markAffectedOutputsStale, which wrote render_version: null on every affected org_chart_outputs row. The column is integer NOT NULL DEFAULT 0 (migration 20260714120000_213), so the UPDATE failed a not-null violation every time — caught, but only logged as a warning (by design: this is best-effort and must never fail the break mutation itself), so the breakage was silent. Effect: marking or unmarking a page break never actually invalidated a finalized output — download, share, embed, and the WCAG report kept serving the pre-break HTML forever. The live preview and the paginated PDF were unaffected because they always render fresh from page-breaks-loader.ts, not from a frozen row. Fixed by writing render_version: 0 instead — isStale (org-chart-share.ts) and the staleness check in regenerateStaleOutputInPlace (org-chart-export.ts) both already treat 0 as falsy/stale, so no other change was needed. Tests updated to mock the column’s NOT NULL constraint (rejecting a null write) so this pattern can’t silently return.