fix: summary modal not appearing when clicking the button in kanban card #219
Conversation
WalkthroughThis change restructures how the AgentInfoPanel component renders content for non-backlog cards, transitioning from a single div container to a fragment wrapper, repositioning the currentPhase badge alongside the model badge, and consolidating the Summary section inline with improved structural organization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical bug where the summary modal failed to appear when triggered from a Kanban card. The fix involves restructuring the rendering of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where the summary modal was not appearing when its button was clicked on a Kanban card. The fix correctly moves the SummaryDialog component to be rendered alongside the button that triggers it, ensuring it's available in the DOM when needed. The change is logical and effectively resolves the bug. I've added one suggestion to improve the key usage in a list rendering for better stability.
| </div> | ||
| <div className="space-y-0.5 max-h-16 overflow-y-auto"> | ||
| {agentInfo.todos.slice(0, 3).map((todo, idx) => ( | ||
| <div key={idx} className="flex items-center gap-1.5 text-[10px]"> |
There was a problem hiding this comment.
Using the array index as a key can lead to performance issues and bugs with dynamic lists. Since todo.content is treated as unique in the parsing logic, it should be used as the key to provide a stable identity to each element.
| <div key={idx} className="flex items-center gap-1.5 text-[10px]"> | |
| <div key={todo.content} className="flex items-center gap-1.5 text-[10px]"> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx (2)
207-219: Consider simplifying event handlers.The expand button uses three different event handlers (
onClick,onPointerDown,onMouseDown) all callingstopPropagation(). TheonPointerDownhandler should be sufficient for modern browsers as it handles both mouse and touch events. Consider simplifying to:onClick={(e) => { e.stopPropagation(); setIsSummaryDialogOpen(true); }}The current implementation works but the redundant handlers add unnecessary code.
🔎 Proposed refactor
<button onClick={(e) => { e.stopPropagation(); setIsSummaryDialogOpen(true); }} - onPointerDown={(e) => e.stopPropagation()} - onMouseDown={(e) => e.stopPropagation()} className="p-0.5 rounded-md hover:bg-muted/80 transition-colors text-muted-foreground/60 hover:text-muted-foreground shrink-0" title="View full summary" data-testid={`expand-summary-${feature.id}`} > <Expand className="w-3 h-3" /> </button>
207-219: Add accessible label for screen readers.The expand button is icon-only and relies solely on the
titleattribute for its description. Consider adding anaria-labelto improve accessibility for screen reader users:aria-label="View full summary"🔎 Proposed improvement
<button onClick={(e) => { e.stopPropagation(); setIsSummaryDialogOpen(true); }} onPointerDown={(e) => e.stopPropagation()} onMouseDown={(e) => e.stopPropagation()} className="p-0.5 rounded-md hover:bg-muted/80 transition-colors text-muted-foreground/60 hover:text-muted-foreground shrink-0" title="View full summary" + aria-label="View full summary" data-testid={`expand-summary-${feature.id}`} > <Expand className="w-3 h-3" /> </button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx (3)
apps/ui/src/lib/agent-context-parser.ts (2)
formatModelName(35-40)DEFAULT_MODEL(30-30)apps/ui/src/lib/utils.ts (1)
cn(5-7)apps/ui/src/components/views/board-view/components/kanban-card/summary-dialog.tsx (1)
SummaryDialog(23-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
apps/ui/src/components/views/board-view/components/kanban-card/agent-info-panel.tsx (5)
131-154: Good structural improvement!The change from a single container div to a fragment wrapper is appropriate and allows better flexibility in rendering. The conditional rendering of the phase badge (line 139) prevents displaying empty badges when
currentPhaseis undefined.
157-195: LGTM!The task list progress section is well-structured with proper status indicators, limited display for better UX (first 3 tasks), and clear visual feedback for different task states.
198-244: Well-structured summary section!The conditional rendering handles both summary display and the no-summary fallback gracefully. Good use of
line-clamp-3for truncation and proper spacing/borders for visual separation.
247-253: Core fix for the modal issue!Rendering the
SummaryDialogwithin the same fragment as the expand button ensures the dialog component is mounted in the React tree when the button is clicked. This properly addresses the PR objective where the modal was not appearing.The comment at line 246 correctly documents the intent.
260-272: Defensive coding for edge case handling.This fallback ensures the
SummaryDialogremains mounted even ifagentInfotemporarily becomes null during a re-render (e.g., during loading state transitions). This prevents the dialog from unmounting unexpectedly if a user has already opened it.While this adds a small overhead, it's good defensive coding that handles edge cases where state might be in flux during async data loading.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.