Skip to content

fix(plans): make the inline schedule preview a live read-only panel - #1177

Merged
cristim merged 5 commits into
mainfrom
feat/inline-schedule-preview
Oct 20, 2026
Merged

cristim merged 5 commits into
mainfrom
feat/inline-schedule-preview

Conversation

@cristim

@cristim cristim commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

Part of #1153 (PR1 of 3). The Create form already shows an inline schedule preview. This PR makes that panel a proper live, read-only preview. Edit mode, the layout and removing the Preview schedule dialog follow in PR2 and PR3.

  • b99cdf1d refactor(plans): extracts buildLocalScheduleIntent into plan-schedule-intent.ts (PlanScheduleIntentInput), the same names the parked feat(plans): plan editor hardening deferred from #631 (atomic Save, editor API, managed dates) #1057 editor branch uses. The panel and the dialog both import it, so the dialog can be deleted later without touching the panel.
  • fd520831 fix(plans): in read-only mode the panel:
    • Live region: has one polite role=status region that carries either the summary ("Target 80% in 4 steps, … (UTC)") or the validation message. The old role=alert is dropped in read-only mode.
    • Invalid input: clears the plot and shows the message. The plot host keeps a fixed minimum height, so the layout does not jump.
    • Rebuilds: rebuilds after a ramp radio change even when it started invalid, which fixes a stuck preview. Only the ramp radios and the four schedule inputs trigger a rebuild; name, term and payment do not, since they do not change the schedule.
    • Clutter removed: hides the five permanently disabled date controls and drops the roadmap sentence.
    • Accessibility: labels the scrollable plot as role=region, described by the status line.
  • 2c8fc969 test(e2e): updates the Create assertions in plan-schedule-preview.spec.ts to match. The dialog assertions are unchanged.

The editable mode (editableDates, used only by tests today) keeps its behaviour. #1153 records that it is deferred to #1057.

Size

Counted as added + deleted from git diff --numstat origin/main...HEAD, tests and e2e included. 3 commits: +75/-35, +78/-15, +9/-7.

Verification

Evidence is posted as PR comments.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added local schedule generation for weekly, preset, and custom payment schedules.
    • Schedule previews now update as settings are entered and provide status summaries after changes.
    • Added clearer read-only messaging and hidden unavailable editing controls.
    • Improved accessibility with live status announcements and clearer preview layout.
  • Bug Fixes

    • Invalid schedule inputs are rejected without leaving stale plots, and valid input restores the preview.
    • Schedule dates now handle UTC boundaries and year transitions correctly.
    • Unrelated setting changes no longer trigger unnecessary schedule rebuilds.

cristim and others added 3 commits October 20, 2026 03:50
The inline panel imported buildLocalScheduleIntent from the dialog module,
which is slated for removal. Move it and its input type to
plan-schedule-intent.ts with their own tests so the dialog can go without
breaking the panel.

Part of #1153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The read-only panel kept stale plots on invalid input, ignored a ramp
radio change after an invalid start, rebuilt on unrelated fields and
showed five disabled controls plus a roadmap sentence. Rebuild only on
the ramp radios and the four schedule inputs, clear the plot and show the
message when the input is invalid, hide the unused controls, and announce
one polite status (summary or validation message) updated on change while
the plot follows input. The scroll area is a described region with a fixed
minimum height so the layout does not jump.

Part of #1153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Create modal no longer shows the roadmap sentence, disabled date
controls or a local-change counter. Assert the hidden controls, the
described scroll region and the live-region summary instead, so the spec
follows the panel cleanup.

Part of #1153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added effort/m Days impact/many Affects most users priority/p2 Backlog-worthy severity/low Minor harm type/feat New capability urgency/this-sprint Within the current sprint labels Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 4 billable files and costs up to $1.00.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 49 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 85 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 64822bd2-2047-4c12-9e3c-b3c3eeb714d2
📥 Commits

Reviewing files that changed from the base of the PR and between 2c8fc96 and 4383ae7.

📒 Files selected for processing: 4
  • frontend/src/__tests__/plan-editor-panel.test.ts
  • frontend/src/plan-editor-panel.ts
  • frontend/src/styles/plans.css
  • frontend/tests-e2e/plan-schedule-preview.spec.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: c22da484-648d-479e-976b-d743ba3abafd
📥 Commits

Reviewing files that changed from the base of the PR and between bb2d231 and 2c8fc96.

📒 Files selected for processing: 7
  • frontend/src/__tests__/plan-editor-panel.test.ts
  • frontend/src/__tests__/plan-schedule-intent.test.ts
  • frontend/src/plan-editor-panel.ts
  • frontend/src/plan-schedule-intent.ts
  • frontend/src/plan-schedule-preview.ts
  • frontend/src/styles/plans.css
  • frontend/tests-e2e/plan-schedule-preview.spec.ts

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The plan preview now uses a shared schedule-intent builder. The editor displays live schedule status when dates are not editable, hides date-editing controls, and updates the preview based on schedule-setting changes.

Changes: Plan schedule preview

Layer / File(s) Summary
Shared schedule intent
frontend/src/plan-schedule-intent.ts, frontend/src/plan-schedule-preview.ts, frontend/src/__tests__/plan-schedule-intent.test.ts
The preview uses a shared input type and schedule-intent builder. The builder validates schedule settings and generates dates and cumulative shares. Tests cover presets, custom schedules, invalid inputs, and UTC date handling.
Read-only editor preview
frontend/src/plan-editor-panel.ts, frontend/src/__tests__/plan-editor-panel.test.ts, frontend/src/styles/plans.css, frontend/tests-e2e/plan-schedule-preview.spec.ts
The editor adds read-only schedule status and hides date-editing controls when dates are not editable. Schedule-setting changes trigger preview rebuilds in read-only mode. Tests cover preview updates, status announcements, and accessibility attributes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 2c8fc

The live Create preview appears mergeable after normal checks; no outstanding issue was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: converting the inline schedule preview into a live, read-only panel.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

cristim and others added 2 commits October 20, 2026 04:00
The status node was both the visible summary and the live region and was
only written on change, so while typing a recovered plot sat next to the
old error text and a valid edit showed a stale target. Keep a visible
summary updated on every build and a separate visually hidden polite
status updated on change only. Hide the plot caption and region while the
input is invalid. Rewrite the no-rebuild test so a rebuild would be
observable, and drop a redundant e2e assertion.

Part of #1153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The scroll region sets display:flex, which overrides the hidden
attribute, so an invalid input left an empty bordered 372px box visible.
Add a [hidden] rule and an e2e check that clearing coverage hides the
region and caption and a valid value restores them.

Part of #1153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim

cristim commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

Merge gate at 4383ae731737d74e00cb32f4b686fb5c3c1b3c13

Two independent reviewers (cold brief, non-author) went through three passes each. Both approve 4383ae73.

Pass 1 (at 2c8fc969): both reviewers requested changes.

  • The status element was both the visible summary and the live region, and it was written only on change. While the user typed, the plot changed next to a stale target or an old error. Both reviewers reproduced this with input-only events.
  • The "settings that do not affect the schedule never rebuild" test was vacuous. Adding plan-term to the trigger ids, or making the filter a no-op, left every test green.
  • The caption and an empty region stayed visible on invalid input.

Pass 2 (at 9ad53564): 9ad53564 adds a visible summary rewritten on every build and a separate sr-only role=status node updated only on change. aria-describedby now points to the visible summary. The rebuild test is rewritten and both surviving mutants now fail. Both reviewers then found that .schedule-preview-scroll { display: flex } overrode [hidden], so the empty 372px box still showed in a real browser.

Pass 3 (at 4383ae73): #plan-editor-panel .schedule-preview-scroll[hidden] { display: none; } (specificity 1,2,0) hides the region. A new Playwright assertion checks that clearing coverage hides the region and caption and that a valid value restores them. It failed before the CSS change and passes after.

Mutation probes killed across the passes:

  • always or never announcing;
  • no plot clear, no invalid message;
  • removed role=region, aria-describedby or role=status;
  • controls not hidden, role=alert restored;
  • the !editable || clause dropped;
  • weekly preset [25,6], Math.ceil step count;
  • plan-term added to the trigger ids, filter made a no-op;
  • summary not written on success, build(true) on input.

Edge cases checked: coverage 0 and 100; step percent 1 (100 steps) and 100; interval 365; notification days 1, 30 and 31; preset over stale custom fields; remount with no duplicate regions or leaked listeners; no API calls from the panel.

Local verification (macOS, exact head, worktree clean):

  • npm ci and npm run build pass. The build warns that app.js is at 501 KiB, just over webpack's advisory 500 KiB limit; PR3 removes the dialog.
  • npm run lint passes with 0 errors.
  • npx jest: 109 suites, 3470 passed, 1 skipped.
  • tests-e2e/plan-schedule-preview.spec.ts passes 4/4 at 1440, 1280, 390 and 320 px.
  • Screenshots of the Create modal at those widths, plus the invalid state at 1440, were reviewed by the implementer, both reviewers and me. They show the live summary above the staircase with no disabled controls or roadmap text, and the error message with no empty box in the invalid state. The layout (form in the narrow right column, preview above the form on phones) is unchanged here; PR2 changes it.

Threads: none open. CodeRabbit is waived as a merge gate.

@cristim
cristim merged commit 0a1f978 into main Oct 20, 2026
28 checks passed
@cristim
cristim deleted the feat/inline-schedule-preview branch October 20, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/many Affects most users priority/p2 Backlog-worthy severity/low Minor harm type/feat New capability urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant