fix(meeting-form): merge Done and Add another mode into one click - #561
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
a57d08c to
a8bdb6c
Compare
"Add another mode for other days" now sits under the open recurrence editor and does both halves itself: it collapses this meeting's schedule into its summary card and opens the new schedule's fields below it. The separate "Done" button that used to collapse the editor first is gone, and the trigger carries its gate instead -- it appears only for a weekly series with at least one weekday and a readable time range, which is what the card summarises and the linked row inherits. A blocked add now disables the trigger and explains it rather than replacing it, so the note reads as the reason a control is off.
70331e9 to
356b63d
Compare
@coderabbitai summary
Description
Adding a second linked schedule used to take two clicks that were really one intention. First Done under the recurrence editor, which collapsed the meeting's own schedule into its summary card; only then did "Add another mode for other days" appear, which opened the second schedule's fields. Done did nothing else — it saved nothing, gated nothing about the payload, and was invisible unless you already knew a second schedule was coming. So the form asked the admin to confirm a schedule they had not been told anything was going to happen to.
Now one click does both halves. "Add another mode for other days" sits under the open recurrence editor, and taking it collapses that editor into its summary card and opens the new schedule's card below it in the same step. Done is gone. Nothing about what gets submitted changes — this is purely the composition flow.
isConfirmed, so it is reachable from the still-open editor. It carries the gate Done used to imply instead: it renders only for a weekly series with at least one weekday selected and a readable time range (scheduleInstants !== null) — exactly what the summary card has to print and what the linked row inherits, so there is never a click that would collapse into a blank card. A blocked add (addBlockedNote) now disables the trigger and shows the note beneath it, wired witharia-describedby, rather than replacing the trigger with the note; a bare warning about a schedule nobody had asked for read as an unprompted error, whereas a disabled control plus a reason reads as an explanation.onConfirmprop and the Done button/row it rendered, plus its now-dead styles. The recurrence editor goes back to being only recurrence controls.onConfirm. Both hosts still passisConfirmed— the collapsed-card state and its Edit this schedule link are unchanged, only the thing that sets it moved.startLinkedDraftnow also setsisScheduleConfirmed, which is what merges the two steps.isScheduleConfirmedstays purely a display state.not-allowed, no hover).click("Done")step; assert the one-click behavior directly (before the click: no summary text, no draft; after it: both). New cases cover the two new withholding conditions — a weekly pattern with no day selected, and an unreadable Date/Time — and the blocked case now assertstoBeDisabled()instead of absence.Testing
yarn test:allfromfrontend/(lint, lint:css, typecheck, unit, component, integration, e2e) — green.yarn test:component MeetingSchedulesandyarn test:e2e tests/e2e/20-linked-meeting-modes.spec.ts."Done",onConfirm,confirmButton/confirmRow, "collapses ... then"). All updated; the only remainingonConfirmhits are unrelated modal props (Suspend/Resume/delete confirmations).Area(s) Touched
Product areas
Integrations
Engineering
Pre-merge Checklist
yarn lint).Stacked on #560 — base branch is
fix/linked-schedule-recreate-guard, notmaster. Review the diff against that base rather than the full stack.