[IAmJSD]

The ladder planned-purchase timeline's Chart.js instance (with its ResizeObserver and listeners) stayed alive after leaving Settings > Purchasing, and opening the General sub-tab even built a new chart inside the hidden Purchasing panel. This PR disposes the chart when navigating away and rebuilds it on return. It is the last open item of the issue: the 409 reload action, focus restore after save and the canvas tab stop shipped in #688, as the maintainer's comment on the issue notes.

Closes #667

What changed

• frontend/src/ladder-timeline.ts: new exported disposeLadderTimeline() destroys the chart, clears the reference and bumps the load generation so pending loads cannot recreate it. initLadderTimeline uses it, and returns early without fetching or building when its root is inside a .hidden section.
• frontend/src/navigation.ts: switchTab calls disposeLadderTimeline() for any target other than Settings, after the unsaved-changes guard, so a cancelled leave keeps the chart. switchSettingsSubTab calls it for any sub-tab other than Purchasing.
• No new re-init code: returning to Purchasing already runs loadGlobalSettings -> initLadderingSettings -> initLadderTimeline.
• Tests in frontend/src/__tests__/navigation.test.ts and frontend/src/__tests__/ladder-timeline.test.ts; new frontend/tests-e2e/ladder-timeline-teardown.spec.ts.
• One commit: 438b140e fix(ladder): dispose the planned-purchase chart when leaving its section.

Why this approach

• Navigation calls one exported dispose function, the teardown hook in switchTab the issue comment asks for, instead of a general per-section lifecycle registry, which would be a larger refactor.
• Re-init reuses the existing settings load path, so there is still only one place that builds the timeline.
• The hidden-section guard is needed because otherwise the General sub-tab would rebuild the chart right after it was disposed. As a side effect, the General sub-tab no longer fetches ladder tranches for a panel nobody can see.

Screenshots

No visible change: disposing the chart does not change what is rendered. The evidence is the live ResizeObserver count in the new e2e test.

Verification

Run on this branch's head 438b140e on top of main a14dc8f2:

• Lint: 0 errors (142 pre-existing warnings). Typecheck clean, build OK.
• Jest: 109 suites, 3490 passed, 1 skipped. --randomize on navigation + ladder-timeline: 107 passed.
• New jest tests: navigation.test.ts "ladder timeline teardown" (leaving Settings disposes the chart, but not while the unsaved-changes dialog is pending; General/Accounts/Users sub-tabs dispose it; Purchasing reloads settings). ladder-timeline.test.ts "chart teardown on navigation" (dispose destroys the chart and a pending reload resolving afterwards does not rebuild it; init after dispose builds a new chart; a hidden section fetches nothing and destroys the previous chart).
• New Playwright ladder-timeline-teardown.spec.ts tracks live ResizeObserver targets inside #ladder-purchase-timeline through Purchasing, General (0 live, no tranche refetch), Purchasing (rebuilt), Plans (0) and browser Back (rebuilt). Against main's build it fails at the General step (1 live observer, expected 0); it passes on this branch.
• Full Playwright (e2e-local.sh): 158 passed.
• Secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Notes for review

• Behavior change: on the General sub-tab the ladder timeline no longer loads at all (before, it loaded invisibly).
• Not handled: if the single-event editor modal is open and the browser Back button is used, the teardown does not close the modal. That was already the case before and is outside the chart-cleanup item.

Overlaps

• fix(frontend): ladder planned-purchases block has no layout styling and locale date inputs #753 also edits ladder-timeline.ts. The source merges cleanly; only the appended tests at the end of ladder-timeline.test.ts conflict (keep both).
• fix(frontend): low-severity copy, toast, confirm and stale-state fixes #728 (refresh the timeline after kill-switch/config save) conflicts in ladder-timeline.ts on the disposeTimeline line. Keep both: disposed = true; generation++; chart?.destroy(); chart = undefined; updateTimeline = undefined;. Its test file has an append-only conflict.
• fix(frontend): ARIA tab pattern has no keyboard model and route changes keep focus #730 also edits navigation.ts but merges cleanly.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 61be5b6
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 534744fc-522e-44ae-b5a4-c9e4fbae8d80

Commits

Reviewing files that changed from the base of the PR and between 61be5b6 and 438b140.

Files selected for processing: 5

• frontend/src/__tests__/ladder-timeline.test.ts
• frontend/src/__tests__/navigation.test.ts
• frontend/src/ladder-timeline.ts
• frontend/src/navigation.ts
• frontend/tests-e2e/ladder-timeline-teardown.spec.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 438b140ee698e8c4ddfc24c7c824bfe59ced0034

Blocking:

• none
  Non-blocking:
• frontend/src/ladder-timeline.ts:28: the hidden guard only detects .hidden ancestors, but leaving Settings hides #admin-tab by removing .active, not by adding .hidden. If a Purchasing loadGlobalSettings is still in flight when the user switches to another main tab (navigation.ts:167 disposes first), initLadderTimeline resolves afterwards and rebuilds the chart inside the inactive tab. It leaks until the next switchTab. This is narrow, but it means the "dispose on leave" guarantee has a race.
• frontend/src/tests/ladder-timeline.test.ts:374: destroy was already called by load() on the reload click before disposeLadderTimeline(), so this assertion would pass even if dispose did not destroy. The second test (:389) does cover dispose, so this is coverage-neutral, but the first test does not prove what its name says.
• The navigation test never exercises the confirmed dirty-guard path (skipDirtyGuard recursion). The code path is the same as the plain switch, so this is low risk.
  Attacked: Confirmed that fix(ladder): timeline editor 409 reload, focus restore and canvas tab stop #688's items (Reload-and-discard on 409, focus restore, no canvas tabindex) are on main a14dc8f, so "Closes fix(ladder): timeline editor 409 recovery, focus and chart cleanup #667" is accurate. Traced the re-init path (switchSettingsSubTab -> loadGlobalSettings -> initLadderingSettings -> initLadderTimeline), including riexchange's switchTab('admin', {skipDefaultLoad}) followed by switchSettingsSubTab('purchasing'). Checked that dispose bumps the generation and sets disposed so pending loads and the editor's onSaved -> load() cannot rebuild the chart, and the Purchasing -> General -> Purchasing ordering of dispose versus the .hidden toggle.

[IAmJSD]

Local verification at 438b140e merged onto current main (84a7f35c; merge-tree f6883633; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree 89dd34d8, main plus 46 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 3914 passed, 3915 total), and the full Playwright suite (208 passed).
• CI is green on 438b140e (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 438b140e.