Gantt strangler: the seam, the adapter, and a correction about what is being replaced - #116
Merged
Merged
Conversation
…s being replaced
Starts the port of GanttCanvasV3 (4,078 lines) to the design-system canvas
(499 lines). Nothing changes for anyone: /gantt-tool renders exactly what it
rendered before unless the URL asks for the replacement by name.
THE CORRECTION FIRST
The agreed sequence was "drag/resize first, then dependencies, capacity,
milestones, cost", on the understanding that a swap would delete those. Two of
those five do not exist, which was checked rather than assumed:
- No drag-to-move and no drag-to-resize on bars. No pointer gesture anywhere
changes a date. `grep` for anything writing a start or end date from a
gesture returns nothing. Dates are edited by double-clicking through to
EditTaskModal / EditPhaseModal.
- No dependency editing, and no dependency rendering either. The canvas
contains zero references to `dependencies`. The field is persisted and read
by exactly two consumers, both deletion-impact modals.
- The four drag/drop handlers on bars are for dropping RESOURCES onto them,
not for moving them in time. Three of the four are inert: they read the
dataTransfer and call logger.warn without assigning anything. Only the
task-level handler reaches updateTask.
- moveTask, movePhase, autoAlignTask and autoAlignPhase in the store have no
callers anywhere in the app.
What does exist and does have to be ported: milestones, the resource capacity
panel, resource drag-assignment onto tasks, search and column configuration,
AMS chevrons, holiday and weekend shading, zoom modes, and the WBS third level.
Cost gating is one prop threaded to one modal's showAdvancedOptions.
WHAT THIS COMMIT ADDS
- canvas-flag.ts — `?canvas=next` opts in, `?canvas=legacy` opts out, the
choice sticks per browser. Legacy is the default and unknown values resolve
to legacy, so the replacement cannot become the default by accident. A URL
rather than a build-time NEXT_PUBLIC_ flag because comparing both canvases
on the same plan in the same session is the whole point of a strangler, and
Vercel inlines NEXT_PUBLIC_ at build time.
- adapter.ts — the only translation between the store's shape and the
canvas's, kept pure so parity can be asserted. It takes the timeline bounds
rather than recomputing them: the origin rule has real subtleties (a 21-day
AMS chevron buffer, non-AMS end dates only) and two implementations would
drift. Durations are inclusive, matching the `+ 1` at every legacy position
site — without it every bar in the plan is a day short.
- GanttCanvasNext.tsx — the replacement wired to the live store, and the
first slice: committing a Move. Move is a good first slice precisely
because it replaces nothing. The legacy canvas cannot change a date from
the canvas at all, so this can only add, and the regression risk on the
legacy path is nil.
One deliberate behavioural difference, documented in the adapter: legacy sizes
the timeline as a percentage of the viewport, so a day means a different
distance in a three-week plan than in a three-year one. The new canvas uses a
fixed px-per-day and scrolls. Constant day width is what makes bars comparable
and what lets it window 1,200 rows rather than squeeze them.
22 new tests. Full suite 2095 passed, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…he focus-trap suite The `validate` job failed with three focus-trap tests timing out at 5s. The cause was not the strangler work in this branch — it was the getClientRects shim added in #113, and specifically its ancestor walk, which called the real getComputedStyle once per ancestor per candidate. `tabbable` consults the shim for every candidate and candidates share ancestors, so the cost compounds over a single query. Measured on the 27 focus-trap tests: with the full-cascade ancestor walk 21.4s without it 7.4s Locally that passed; under coverage instrumentation it slowed enough to push three tests past the per-test timeout. CI red, local green — which is exactly the discrepancy that makes people stop trusting a suite, so the fix is in the shim rather than in the timeout. The element's own style keeps the full cascade check. Ancestors are now checked with cheap signals only: the `hidden` attribute and inline `style.display === 'none'`. That still catches how anything in this repo hides a subtree — Vitest does not load CSS Modules, so no stylesheet rule can set display:none on an ancestor in this environment. The comment says to revisit if that changes rather than let the shim quietly under-report. Because this deliberately WEAKENS the check, what remains is now asserted rather than assumed: tests/__tests__/get-client-rects-shim.test.tsx pins down that visible elements are tabbable, detached ones are not, own display:none / visibility:hidden still hide, a display:none or [hidden] ancestor still hides a buried control (the collapsed-accordion shape), and an ancestor with mere opacity/overflow does not. `tabbable` becomes an explicit devDependency, pinned to the version already in the tree via focus-trap — the test depends on its judgment, which is the judgment the shim exists to satisfy. Coverage-mode suite (what CI runs): 2102 passed, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
The next canvas now draws every milestone as the legacy one does — a 2px
half-opacity rule through the full canvas height and a marker at the top that
opens the milestone modal — with the same day arithmetic as the bars, the same
default colour (#FF3B30), and the same out-of-range drop. The modal itself is
the legacy MilestoneModal reused verbatim, wired to the same store actions
including the alert() on failure, which is not this slice's to redesign. The
page's milestone toolbar button works behind ?canvas=next because the next
canvas honours the same showMilestoneModal prop pair the legacy one takes.
Three deliberate changes from the legacy rendering, each named in the
component:
- The marker is a real <button> whose accessible name carries the
milestone's name and date. Legacy used a div with role="button" and only
a title, which reads as "button" once per milestone.
- The markers do NOT live in the axis, although visually they sit at the
top of the canvas: the axis is aria-hidden (it is a scale, not data), and
a focusable control inside an aria-hidden subtree is reachable by Tab but
invisible to assistive technology — the worst of both.
- The rule is aria-hidden decoration with pointer-events off, so a rule
crossing a bar never steals the bar's click, and the same fact reaches a
screen reader once through the button rather than once per rule.
One ARIA-structure trade is made knowingly and documented at the render site:
the layer sits inside the treegrid element, whose strict contract wants only
rows as children. The alternative — an overlay outside the treegrid synced to
two scroll axes — reintroduces exactly the pane-drift machinery this canvas
was built to avoid. What is compromised is validator purity, not
reachability.
The adapter grows toCanvasMilestones, which reuses dayOffset so a milestone
and a bar on the same date can never disagree by a day, and normalises the
store's persisted color: "" to undefined so the default actually applies.
Out-of-range milestones are kept by the adapter and dropped by the layer —
the same split the bars use, and per-marker, so one stray date does not hide
the rest. The boundary is inclusive: a go-live milestone on the plan's final
day renders.
12 new tests (8 layer, 4 adapter). Full suite under coverage, as CI runs it:
2114 passed, 0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
The next canvas's axis was rendering unlabelled — GanttCanvasNext passed no
ticks, so the timeline was a ruler with no numbers. That was a worse gap than
the missing shading, so this slice closes both.
next/axis.ts is pure, like the adapter, and shares its dayOffset — so the
only way a tick can drift off a bar's date is if an asserted function
changes. Legacy's single labelled row (primary over secondary, "W28" over
"06-Jul-26") maps directly onto the canvas axis's two rows:
grain minor (lower) major (upper)
Day "05" "Jan 26"
Week "W28" "Jan 26"
Month "Jul" "2026"
Quarter "Q3" "2026"
One rule worth knowing: date-fns interval helpers return period STARTS, and
the first period usually starts before the plan does — a plan starting 5 Jan
gets its "Jan" tick at day -4, which the axis clips into invisibility. Ticks
before day 0 clamp to 0, which is where the legacy percentage layout
effectively pinned them.
Shading uses the same getUnifiedHolidays and the same hardcoded "ABMY"
region default as the legacy canvas, so both canvases shade identical days
on the same plan. Weekends are walked a day at a time — a few thousand
iterations at worst, cheaper than being clever. A holiday landing on a
weekend keeps its name rather than being swallowed by anonymous weekend
shading. The canvas already gates shading to Day and Week grains, where a
day is wide enough to be information rather than texture.
11 new tests, including the two clamps (mid-month start, mid-week start),
the weekend/holiday name collision, and out-of-plan holidays being dropped
rather than shading a phantom day. Full suite under coverage: 2125 passed,
0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
…isted to fix
This was not visual polish. getProjectDuration() deliberately excludes AMS
end dates so a three-year support contract does not stretch a four-month
plan, but the adapter still measured an AMS phase's bar to its contract end —
so behind ?canvas=next an AMS phase drew a bar overrunning the canvas by up
to three years' worth of pixels. The legacy canvas never had this problem
because it paints AMS phases as a fixed 160px strip of five chevrons anchored
at the start date.
The port reproduces that:
- BarPlacement gains `ams?: boolean`, documented as "durationDays is
ignored for these". The adapter sets it for phaseType === "ams" and for
task.isAMS, placing both at the start with a placeholder duration of 1 —
a placeholder, not a claim, and asserted in the tests to never exceed
totalDays.
- GanttAmsChevron (ds/gantt) keeps legacy's exact geometry — five 24×32
chevrons, 4px gaps, #FF6B35, 0.7 rest opacity — but mirrors GanttBar's
interactive contract instead of legacy's div: one <button> whose
accessible name carries the whole fact ("Hypercare, ongoing support
contract, starts 1 Mar 26"), aria-pressed for selection, select and
keydown delegated. The canvas's cursor, selection and announcement
plumbing cannot tell an AMS row from any other; only the paint differs.
The five SVGs are aria-hidden — five decorative shapes announced
individually would bury the one named button between them.
- GanttCanvas branches on placement.ams at the render site and nowhere
else.
7 new tests (5 chevron, 2 adapter — including the overrun assertion). Full
suite under coverage: 2132 passed, 0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
The legacy tree pane is a data grid — name, calendar duration, working days,
start-end dates — and that is reading parity, not chrome: a plan is read off
those columns. The next canvas's tree was name-only.
Also a correction to the slice list, found by reading rather than assuming:
there is no task search in the legacy canvas. The Search input filters the
RESOURCE list inside the capacity panel, so "search" belongs to that slice
and is struck from this one.
next/details.ts computes the three cells per row with the same three
functions the legacy canvas calls and the same output formats — "3.2 m",
"42 d", "05-Jan-26 (Mon) - 30-Jan-26 (Fri)" — asserted literally in tests,
because whatever this produces is what sits beside the legacy numbers when
the two canvases are compared on one plan. Working days share the holiday
list with the axis shading, so the count a row reports and the days the
canvas shades can never disagree. AMS rows are computed uniformly, exactly
as legacy does: the timeline excludes contract ends so the canvas does not
stretch, but the cell describes the contract, and those are allowed to
differ.
The canvas takes details as an optional prop, pre-formatted: when present,
the tree pane renders legacy's grid at legacy's default widths (280 + 90 +
90 + 180) under legacy's header labels (Duration, Work Days, Start-End);
when absent, every pre-existing caller keeps the name-only 280px tree,
asserted by test. Column resizing is deliberately not ported yet.
Working days also join each bar's accessible name ("Phase 1, phase, day 0 to
day 29, 20 working days"). The TimelineAxis header comment has promised this
since the canvas was built — the weekend/holiday shading is aria-hidden, and
the bar's name is where that information actually reaches assistive
technology. This slice is what makes the promise true. Rows without a
details entry keep their unchanged name rather than gaining a dangling
phrase.
8 new tests (4 details, 4 canvas). Full suite under coverage: 2140 passed,
0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
The largest remaining piece. The numbers come from calculateResourceCapacity
— the same calculator the legacy panel calls, with the same inputs — so the
two panels can never show different allocations for one plan. The seam
(next/capacity.ts) only reshapes: aligned per-week percentages for the
matrix, and the search haystack, which reproduces legacy's four searchable
fields exactly (name, category label, company, project role) because "search
finds it in one panel but not the other" is the parity break that erodes
trust fastest.
GanttCapacityPanel (ds/gantt) is the matrix AllocationCell was built for:
one row per resource, one cell per project week, figure + fill + hatch so an
over-allocation survives a projector, a printout and colour-blindness. It is
a real <table> — row and column headers reach assistive technology for free,
which is exactly the context an allocation figure needs ("Ada Lovelace, W02,
12 Jan – 18 Jan 26: 120% allocated, over-allocated" is a test assertion).
The name column is sticky so a row keeps its identity forty weeks in; the
search count announces through a live region so a keyboard user hears the
result without leaving the field.
One deliberate difference, documented in the component: legacy pixel-aligns
capacity cells under the timeline; this panel is a self-contained matrix
with its own header and scroll. At Month and Quarter grains a week is 20px
or 8px wide, which cannot hold a figure — pixel alignment and readable
cells are mutually exclusive, and the cell's whole design is that the figure
is always readable.
Not ported yet, stated rather than implied: manual per-week overrides
(legacy holds them in component-local state) and group aggregation rows.
The calculator only runs while the panel is open — for a large plan it
walks every task × week, and computing it closed is pure waste.
11 new tests (5 mapping, 6 panel). Full suite under coverage: 2151 passed,
0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
…ilure CI's validate job went red on the capacity-panel commit with the /gantt-tool first-load budget: 1307.6kB against the 1300kB ceiling. The page imported GanttCanvasNext statically, which shipped the entire replacement — ds canvas, capacity calculator, milestone wiring — to every visitor ALONGSIDE the legacy canvas, even though it only renders behind ?canvas=next. Local runs had not caught it because the budget test measures the real build output and the local .next predated the strangler wiring; CI builds fresh. Reproduced locally against a fresh build (identical number), fixed, re-measured: 23/23 budget assertions pass. The fix is dynamic() with ssr off, and it is the strangler's contract made true at the bundle level rather than just the render level: the replacement costs nothing until the URL asks for it. The budget test is doing exactly what it was rebuilt to do in #113 — this is the first regression it caught. Full suite under coverage, against the fresh build: 2151 passed, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
The app shipped titled "Bound" with a two-wave "≈" mark. It is Cockpit now,
carrying the supplied beacon mark — an outer ring broken at the base, an
inner arc, a gold point, and a stem dropping through the break.
The mark is recreated as vectors (public/logo-cockpit.svg) rather than
embedding the raster, so it is crisp at 16px in a tab and 512px as an app
icon. It was verified by rendering and comparing against the artwork, not by
arithmetic alone — the first pass had strokes 1.7x too heavy, which is
visible the moment you look. Final geometry is documented in the file so
edits keep the proportions.
Surfaces renamed, every one checked rather than assumed:
- layout metadata title, manifest name/short_name
- GlobalNav: the new SVG plus the wordmark text
- /api/favicon: the dynamic favicon now draws the beacon in the status
foreground colour, with a heavier stroke than the full-size mark (34 vs
17 in the 512 viewBox) because at 16-32px the elegant stroke aliases
into fog. The gold point keeps its brand colour except on the amber
"disconnected" background, where gold-on-amber has no contrast and the
foreground colour takes over. The status system itself is untouched.
- WebAuthn rpName (both the lib export and the begin-register literal).
rpID is NOT touched: it is the domain, and changing it would orphan
every registered passkey. rpName is only what the passkey prompt shows.
- Email templates: header, from-name, subjects, five footer copyright
lines, and the APP_NAME fallback.
- Export branding text in export-utils.
public/logo-bound.svg is deleted with its last reference. The stale
logo-cockpit.png and cockpit-icon.png in public/ are left as they were —
unreferenced before and after, and not this change's to adjudicate.
Full suite under coverage: 2151 passed, 0 failed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
Slice investigation, taken to the end rather than to the first plausible
answer. The plan said "port resource drag-assignment"; the code says there
is nothing to port:
- No drag source exists in the working tree: nothing calls
setData("resourceId"), nothing is draggable, nothing binds onDragStart.
The two `div[draggable="true"]` hits are CSS selectors styling a feature
that is not there.
- No drag source has EVER existed. A sweep of all 122 commits in the
repository's history finds no version of any file containing the
setData call the drop handlers read. All four drop handlers in
GanttCanvasV3 — including the task-level one that writes
resourceAssignments — are and always were unreachable.
- The modal that drop opened, TaskResourceModal, does not persist: its
onSave is a literal "// TODO: Implement resource assignment
persistence" that closes the modal and discards the input.
- The one assignment path that does persist (ResourceAllocationModal ->
handleApplyBulkAllocation -> the team-capacity allocations API) writes
weekly allocation overrides — a different data model from
task.resourceAssignments, which the capacity calculator reads for its
per-task breakdown and which has no working UI write path at all.
docs/REMEDIATION.md sec 3.1 is rewritten to carry all of this, replacing the
original "do not swap, that deletes working functionality" warning, which
was substantially wrong: of the features it protected, drag/resize,
dependency editing and drag-assignment never existed.
Making task-level assignment real is a product decision — which of the two
half-wired data models is canonical — and belongs after the flip as its own
feature, not inside a parity port. The dead handlers and ghost CSS in the
legacy canvas are safe to delete in a cleanup commit; they are unreachable,
so removal changes nothing observable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TArsz4CrMDAKmeALMozkR5
ib823
marked this pull request as ready for review
August 8, 2026 05:14
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
2 participants
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.
Starts the port of
GanttCanvasV3(4,078 lines) to the design-system canvas (499 lines). Nothing changes for anyone —/gantt-toolrenders exactly what it rendered before unless the URL asks for the replacement by name (?canvas=next, sticky per browser, unknown values resolve to legacy).What the PR now contains
9f3144dcanvas-flag.ts), the pure adapter, and slice 1: Move commite04dd1dgetClientRectsshim's ancestor walk was 65% of the focus-trap suite's runtime (21.4s→7.3s); under coverage that pushed three tests past their timeout. Narrowed + a 7-test guard suitec28395earia-hiddenaxis3a30381dayOffset, samegetUnifiedHolidays/"ABMY" source as legacy753e22f1bbbdbcTimelineAxis's headerCorrections that reshaped the plan (checked, not assumed)
Searchinput filters the resource list inside the capacity panel — moved to that slice.logger.warn, no write).moveTask/movePhase/autoAlignTask/autoAlignPhasehave no callers.Parity contract
next/adapter.tsis the only store↔canvas translation, pure so parity is asserted rather than eyeballed. Origin fromgetProjectDuration()(passed in, not recomputed). Inclusive durations (the+ 1at every legacy position site). Milestones, ticks, and detail cells reuse the samedayOffset/ holiday list, so nothing sharing a date or a day count can disagree.One deliberate behavioural difference: legacy fits the plan to the viewport width; the new canvas uses fixed px-per-day and scrolls — what makes bars comparable and lets it window 1,200 rows.
Verification
60 new tests across flag, adapter, milestone layer, axis model, AMS chevron, detail columns, and the shim guard. Full suite under coverage, as CI runs it: 2140 passed, 0 failed — the legacy path untouched, and asserted so (no details prop → the exact pre-existing tree).
Remaining slices