Repository navigation
ui(automation): CustomCommands + GuildAutomation redesign with full test coverage - #969
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 33 minutes and 42 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughCustomCommands and GuildAutomation pages replace framer-motion animations and Card-based layouts with surface-panel containers and conditional rendering. CustomCommands converts an animated grid to a collapsible list with category filtering; GuildAutomation removes motion wrappers from the RunCard, result views, and manifest editor. Tests are updated with regex-based queries and accessible role selectors. The shared package now re-exports constants at the top level. ChangesUI Refactoring: Animation Removal and Container Standardization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 |
|
Failed to generate code suggestions for PR |
|
Size Change: -44 B (-0.01%) Total Size: 424 kB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/pages/GuildAutomation.test.tsx (1)
569-585:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh test can pass without actually validating refresh behavior.
At Line 573-576, selecting “first button with an svg” is nondeterministic, and Line 578 allows silently skipping assertions when not found. This can produce false-green tests.
Suggested fix
- const refreshButtons = document.querySelectorAll('button') - const refreshButton = Array.from(refreshButtons).find((btn) => - btn.querySelector('svg'), - ) - - if (refreshButton) { - await user.click(refreshButton) - - await waitFor(() => { - expect(api.automation.getStatus).toHaveBeenCalled() - expect(api.automation.getManifest).toHaveBeenCalled() - }) - } + const refreshButton = screen.getByRole('button', { name: /refresh status/i }) + await user.click(refreshButton) + + await waitFor(() => { + expect(api.automation.getStatus).toHaveBeenCalled() + expect(api.automation.getManifest).toHaveBeenCalled() + })🤖 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 `@packages/frontend/src/pages/GuildAutomation.test.tsx` around lines 569 - 585, The current test uses a nondeterministic DOM query (selecting the "first button with an svg") and silently skips assertions if no button is found; replace that with a deterministic query for the refresh control (e.g., use screen.getByRole('button', { name: /refresh/i }) or screen.getByTestId('refresh-button') targeting the specific refresh button in GuildAutomation.test.tsx), remove the surrounding if conditional so the test fails if the button is missing, perform the click with user.click(refreshButton) and then assert api.automation.getStatus and api.automation.getManifest were called.
🧹 Nitpick comments (1)
packages/frontend/src/pages/CustomCommands.tsx (1)
105-105: 💤 Low valueRedundant border declaration.
The
surface-panelutility already includesborder: 1px solid var(--lucky-border)per the shared CSS. The explicitborder border-lucky-borderclass is unnecessary.🔧 Suggested fix
- <div className='surface-panel rounded-lg p-4 space-y-3 border border-lucky-border'> + <div className='surface-panel rounded-lg p-4 space-y-3'>🤖 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 `@packages/frontend/src/pages/CustomCommands.tsx` at line 105, The div in the CustomCommands component uses redundant explicit border classes on top of the surface-panel utility; remove the extra classes "border" and "border-lucky-border" from the className on the div (the element containing 'surface-panel rounded-lg p-4 space-y-3') so the styling relies on surface-panel's built-in border, leaving other classes intact to preserve spacing and rounding.
🤖 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.
Inline comments:
In `@packages/frontend/src/pages/CustomCommands.tsx`:
- Around line 192-239: The Switch is nested inside the expandable button so its
clicks bubble and also trigger setExpandedCommand; to fix, stop event
propagation on the Switch so toggling only changes state: add an event handler
(e.g., onClick and/or onPointerDown) on the <Switch> element or its immediate
container that calls event.stopPropagation(), and keep the existing
onCheckedChange={() => handleToggle(cmd)} so toggling still works without
expanding/collapsing the row (refer to Switch, handleToggle, setExpandedCommand,
expandedCommand, and cmd.id).
In `@packages/frontend/src/pages/GuildAutomation.tsx`:
- Around line 395-399: The toggle button hardcodes aria-label and doesn't expose
its state to assistive tech; update the button that uses manifestExpanded /
setManifestExpanded to include aria-expanded={manifestExpanded} and make the
aria-label dynamic (e.g., manifestExpanded ? "Collapse manifest" : "Expand
manifest") so the label matches the state; apply the same changes to the other
similar toggle button elsewhere in this component that also toggles
manifestExpanded.
- Around line 83-85: The chevron affordance is only rendered when run.summary
exists, so runs that only have run.error show no expand cue; update the
conditional around ChevronUp/ChevronDown to show when either run.summary or
run.error is present (e.g., use (run.summary || run.error) and keep the expanded
toggle logic intact), and apply the same change to the corresponding expanded
panel rendering block that currently checks run.summary (the area around the
expanded variable and the ChevronUp/ChevronDown usage) so error-only runs also
display the chevron and can be expanded to see details.
---
Outside diff comments:
In `@packages/frontend/src/pages/GuildAutomation.test.tsx`:
- Around line 569-585: The current test uses a nondeterministic DOM query
(selecting the "first button with an svg") and silently skips assertions if no
button is found; replace that with a deterministic query for the refresh control
(e.g., use screen.getByRole('button', { name: /refresh/i }) or
screen.getByTestId('refresh-button') targeting the specific refresh button in
GuildAutomation.test.tsx), remove the surrounding if conditional so the test
fails if the button is missing, perform the click with user.click(refreshButton)
and then assert api.automation.getStatus and api.automation.getManifest were
called.
---
Nitpick comments:
In `@packages/frontend/src/pages/CustomCommands.tsx`:
- Line 105: The div in the CustomCommands component uses redundant explicit
border classes on top of the surface-panel utility; remove the extra classes
"border" and "border-lucky-border" from the className on the div (the element
containing 'surface-panel rounded-lg p-4 space-y-3') so the styling relies on
surface-panel's built-in border, leaving other classes intact to preserve
spacing and rounding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 188f4e80-a8c1-4455-bbd0-cce9eed0c713
📒 Files selected for processing (5)
packages/frontend/src/pages/CustomCommands.test.tsxpackages/frontend/src/pages/CustomCommands.tsxpackages/frontend/src/pages/GuildAutomation.test.tsxpackages/frontend/src/pages/GuildAutomation.tsxpackages/shared/src/index.ts
4247815 to
babf765
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|



Summary
Redesign and test coverage for CustomCommands and GuildAutomation pages.
Tests
Summary by CodeRabbit
Release Notes