Repository navigation
refactor(calendar): de-emphasize agent UX in event modal - #1237
Merged
Merged
Conversation
Replaces the always-visible 'Configure agent trigger' button (and its separate dialog) with an inline collapsed disclosure inside EventModal. Header reads 'Off' or '<agent name> at start' so the modal reads as a normal calendar by default and surfaces which agent is attached when one is, without a second-dialog hop. Moves 'Linked page' under a collapsed Advanced section so casual users see only normal calendar fields. Adds a small Zap indicator on event chips in Month/Week/Day/Agenda/MobileDayAgenda whenever hasAgentTrigger is true, matching the task list's amber Trigger badge. Wires agentTrigger (three-state: undefined no-op, null remove, object upsert) through useCalendarData and CalendarView.handleEventSave to the existing POST/PATCH endpoints. Recurring events render an inline 'can't have agent triggers' note instead of the form. Personal events get no agent affordance at all. Retires EventAgentTriggerDialog + its test in favor of EventModal.test.tsx covering the inline disclosure, Advanced collapse, recurring guard, and SWR pause across remote refetch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR replaces a dedicated agent-trigger dialog with inline configuration within the event modal, removes ChangesAgent Trigger Configuration Migration
Sequence DiagramsequenceDiagram
participant User
participant EventModal
participant SWR as SWR Fetchers
participant API
participant CalendarView
participant CalendarView2 as useCalendarData
User->>EventModal: Opens modal to edit/create event
EventModal->>SWR: Request existing trigger (if updating)
EventModal->>SWR: Request available agents
SWR->>API: Fetch /triggers & /agents
API-->>SWR: Return trigger data & agent list
SWR-->>EventModal: Hydrate agent state from loaded trigger
User->>EventModal: Configure agent (select, edit prompt, toggle on/off)
Note over EventModal: Local agent state updated
User->>EventModal: Click Save
EventModal->>EventModal: Validate (recurring check, required fields)
EventModal->>EventModal: Compute agentTrigger payload
EventModal->>CalendarView: onSave(eventData with agentTrigger)
CalendarView->>CalendarView: handleEventSave extracts agentTrigger
alt Create Event
CalendarView->>CalendarView2: createEvent(eventData with agentTrigger)
else Update Event
CalendarView->>CalendarView2: updateEvent(eventId, eventData with agentTrigger)
end
CalendarView2->>API: POST/PUT event (includes agentTrigger)
API-->>CalendarView2: Event saved with trigger
EventModal-->>User: Modal closes
CalendarView->>User: Calendar view updates with new/modified event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Adds a sr-only DialogDescription to EventModal so the Radix Dialog has an aria-describedby (fixes the missing-Description warning that was emitted on every open). Wires the agent-trigger Switch to its Label via id/htmlFor so the toggle has an accessible name. Defensively keeps agentEnabled=false for recurring events even if a stale trigger row exists, so the broken state cleans itself up on save (agentTrigger=null) instead of trapping the user behind a validation error they can't dismiss. Extends EventModal.test.tsx with three save-path tests: enabled-on-save forwards a complete agentTrigger object, toggle-off-on-existing-trigger forwards null, personal-event save omits agentTrigger entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/calendar/MobileDayAgenda.tsx (1)
212-214: ⚡ Quick winPrefer
sr-onlytext overaria-labelon the inline icon (lucide's own recommendation)Passing
aria-labelto a lucide-react icon removes the defaultaria-hidden="true"and makes the icon visible to screen readers — so it does work. However, lucide's own accessibility guide explicitly recommends against usingaria-labelon icons, preferring the CSS framework's "visually hidden" utility instead, and links to further reading on whyaria-labelmay not be ideal.Since the icon is rendered inside a
<h3>alongside the event title text, it's also worth noting that descriptions should only be provided to standalone icons that aren't purely decorative — providing accessible names to non-functional elements only increases clutter when using screen readers.The recommended pattern for this case is to make the icon decorative and expose the supplemental information via a
sr-onlyspan:♻️ Proposed refactor
- {event.hasAgentTrigger && ( - <Zap className="inline-block h-3.5 w-3.5 mr-1 text-amber-500 align-text-top" aria-label="Agent trigger configured" /> - )} - {event.title} + {event.hasAgentTrigger && ( + <> + <Zap className="inline-block h-3.5 w-3.5 mr-1 text-amber-500 align-text-top" aria-hidden="true" /> + <span className="sr-only">Agent trigger configured. </span> + </> + )} + {event.title}The same pattern applies to
AgendaView.tsx(Line 191) and the other calendar view files updated in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/calendar/MobileDayAgenda.tsx` around lines 212 - 214, Make the Zap icon decorative instead of giving it an aria-label: remove the aria-label prop from the lucide <Zap> usage inside MobileDayAgenda (the conditional using event.hasAgentTrigger) and add a visually-hidden span (e.g., className="sr-only") next to the icon containing the descriptive text like "Agent trigger configured"; do the same refactor for the analogous icon usages in AgendaView.tsx and other calendar view components so icons remain aria-hidden while the supplemental text is exposed via the sr-only span.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/components/calendar/MobileDayAgenda.tsx`:
- Around line 212-214: Make the Zap icon decorative instead of giving it an
aria-label: remove the aria-label prop from the lucide <Zap> usage inside
MobileDayAgenda (the conditional using event.hasAgentTrigger) and add a
visually-hidden span (e.g., className="sr-only") next to the icon containing the
descriptive text like "Agent trigger configured"; do the same refactor for the
analogous icon usages in AgendaView.tsx and other calendar view components so
icons remain aria-hidden while the supplemental text is exposed via the sr-only
span.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f00d991-06f7-48d5-a622-ace65938cd4b
📒 Files selected for processing (12)
apps/web/src/components/agent-triggers/AgentTriggerSection.tsxapps/web/src/components/calendar/AgendaView.tsxapps/web/src/components/calendar/CalendarView.tsxapps/web/src/components/calendar/DayView.tsxapps/web/src/components/calendar/EventAgentTriggerDialog.tsxapps/web/src/components/calendar/EventModal.tsxapps/web/src/components/calendar/MobileDayAgenda.tsxapps/web/src/components/calendar/MonthView.tsxapps/web/src/components/calendar/WeekView.tsxapps/web/src/components/calendar/__tests__/EventAgentTriggerDialog.test.tsxapps/web/src/components/calendar/__tests__/EventModal.test.tsxapps/web/src/components/calendar/useCalendarData.ts
💤 Files with no reviewable changes (2)
- apps/web/src/components/calendar/EventAgentTriggerDialog.tsx
- apps/web/src/components/calendar/tests/EventAgentTriggerDialog.test.tsx
…ggers Aligns the agent-trigger Zap indicator on Month/Week/Day/Agenda/MobileDayAgenda chips to TaskListView's pattern (aria-hidden icon + sr-only text label) so screen readers announce it once with surrounding context, instead of independently via aria-label. Tightens save semantics so a recurring event with a stale trigger row no longer silently deletes the row on every unrelated edit. The new guard (existingTrigger && !isRecurring) makes agentTrigger=null only fire when the user explicitly toggled off a configured trigger they could see, not as a side effect of editing the title. Adds a test pinning that defensive behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai Already addressed in commit 4ac9d73 — the same |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Owner
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
…aries
- Replace single-member union `AgentTriggerSavePayload = | { ... }` with
a plain object type; the leading `|` was syntactic noise.
- Replace nested ternaries in `lastRunStatusFor` and `agentHeaderLabel`
with explicit if/else chains per project standard.
- Collapse the two duplicate-body `else if` branches in the trigger
hydration effect into one combined predicate.
- Extract `firstSavePayload(onSave)` helper in the test file to remove
the repeated `waitFor → cast → access` block from four tests.
No behavior changes. `pnpm --filter web typecheck` clean and all 10
EventModal tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 12, 2026
2witstudios
added a commit
that referenced
this pull request
May 15, 2026
* refactor(calendar): de-emphasize agent UX in event modal
Replaces the always-visible 'Configure agent trigger' button (and its separate dialog) with an inline collapsed disclosure inside EventModal. Header reads 'Off' or '<agent name> at start' so the modal reads as a normal calendar by default and surfaces which agent is attached when one is, without a second-dialog hop.
Moves 'Linked page' under a collapsed Advanced section so casual users see only normal calendar fields. Adds a small Zap indicator on event chips in Month/Week/Day/Agenda/MobileDayAgenda whenever hasAgentTrigger is true, matching the task list's amber Trigger badge.
Wires agentTrigger (three-state: undefined no-op, null remove, object upsert) through useCalendarData and CalendarView.handleEventSave to the existing POST/PATCH endpoints. Recurring events render an inline 'can't have agent triggers' note instead of the form. Personal events get no agent affordance at all.
Retires EventAgentTriggerDialog + its test in favor of EventModal.test.tsx covering the inline disclosure, Advanced collapse, recurring guard, and SWR pause across remote refetch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(calendar): a11y + recurring-trigger defense in event modal
Adds a sr-only DialogDescription to EventModal so the Radix Dialog has an aria-describedby (fixes the missing-Description warning that was emitted on every open).
Wires the agent-trigger Switch to its Label via id/htmlFor so the toggle has an accessible name.
Defensively keeps agentEnabled=false for recurring events even if a stale trigger row exists, so the broken state cleans itself up on save (agentTrigger=null) instead of trapping the user behind a validation error they can't dismiss.
Extends EventModal.test.tsx with three save-path tests: enabled-on-save forwards a complete agentTrigger object, toggle-off-on-existing-trigger forwards null, personal-event save omits agentTrigger entirely.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(calendar): align Zap a11y + don't auto-delete stale recurring triggers
Aligns the agent-trigger Zap indicator on Month/Week/Day/Agenda/MobileDayAgenda chips to TaskListView's pattern (aria-hidden icon + sr-only text label) so screen readers announce it once with surrounding context, instead of independently via aria-label.
Tightens save semantics so a recurring event with a stale trigger row no longer silently deletes the row on every unrelated edit. The new guard (existingTrigger && !isRecurring) makes agentTrigger=null only fire when the user explicitly toggled off a configured trigger they could see, not as a side effect of editing the title.
Adds a test pinning that defensive behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(calendar): tidy event modal — drop noise union, flatten ternaries
- Replace single-member union `AgentTriggerSavePayload = | { ... }` with
a plain object type; the leading `|` was syntactic noise.
- Replace nested ternaries in `lastRunStatusFor` and `agentHeaderLabel`
with explicit if/else chains per project standard.
- Collapse the two duplicate-body `else if` branches in the trigger
hydration effect into one combined predicate.
- Extract `firstSavePayload(onSave)` helper in the test file to remove
the repeated `waitFor → cast → access` block from four tests.
No behavior changes. `pnpm --filter web typecheck` clean and all 10
EventModal tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
EventModal. Header readsOffor<agent name> at start, so the modal reads as a normal calendar by default and surfaces which agent is attached when one is, without a second-dialog hop.Linked pageunder a collapsed Advanced section so casual users see only normal calendar fields (Title / Date / Time / Location / Color / Description). Add a small Zap indicator on event chips inMonthView/WeekView/DayView/AgendaView/MobileDayAgendawheneverhasAgentTriggeris true (using the samearia-hiddenicon +sr-onlytext pattern asTaskListView's amber Trigger badge).agentTrigger(three-state:undefinedno-op,nullremove, object upsert) throughuseCalendarDataandCalendarView.handleEventSaveto the existing POST/PATCH endpoints. Recurring events render an inline "can't have agent triggers" note instead of the form. Personal events get no agent affordance at all.agentEnabled=falseon hydration AND skips sendingagentTrigger=nullon save, so unrelated edits don't silently delete the broken row.DialogDescriptionadded to satisfy Radix's aria-describedby contract; the agent-trigger Switch is now properly associated with its Label via id/htmlFor.EventAgentTriggerDialog+ its test in favor ofEventModal.test.tsx(10 tests covering: collapsed default, hidden for personal events, prefilled when editing, recurring-disabled note + stale-row save defense, Advanced collapse, save-with-trigger payload, save-with-toggle-off → null, personal-save omits agentTrigger, and SWR pause across remote refetch).Why
PR #1232 wired up agent triggers but the resulting UX over-rotated toward agent scheduling: every saved drive event got a full-width "Configure agent trigger" button, "Linked page" read as agent context, and the agent picker was hidden behind two clicks. The user complaint was "is it always an agent event or how do you select which agent? this should still function as a normal calendar."
Test plan
pnpm --filter @pagespace/db build && pnpm --filter @pagespace/lib build(refresh stale dist; the calendar/workflow typecheck noise was caused bydist/schema/{workflow-runs,task-triggers}.{js,d.ts}never being compiled andcalendar-triggers.d.tsdescribing the pre-workflowIdschema)pnpm --filter web typecheckcleanpnpm --filter web lintclean (one pre-existing warning in unrelated file)pnpm --filter web test src/components/calendar— 26 tests pass (16 existing + 10 new EventModal)pnpm --filter web test src/lib/workflows/__tests__/calendar-trigger-helpers.test.ts src/app/api/calendar— 43 tests passpnpm --filter web test src/lib/ai/tools/__tests__/calendar-write-tools.test.ts— 58 AI-tool tests passagentTrigger, event saves with trigger attached<agent name> at start; expand prefills sectionagentTrigger: null, trigger removedhasAgentTrigger=trueshow small Zap icon🤖 Generated with Claude Code