fix(transactions): remove duplicate close button on transaction detai…#65
Conversation
…l modal - Remove redundant manual × button conflicting with DialogClose primitive - Refactor Badge risk level logic: extract getRiskLevel() call into const riskLevel - Replace ternary chain with static riskStyles Record map at module scope Closes DjedAlliance#64
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces a typed Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/app/transactions/page.tsx`:
- Around line 31-35: The modal uses selectedTransaction?.risk while the list
derives risk from amountSC, causing inconsistent styling; refactor to compute
the same risk level used by the list and reuse the riskStyles mapping for the
dialog. Locate where amountSC→risk is calculated (the same logic used when
rendering rows) and extract or call it from the detail modal so you compute a
riskLevel (e.g., getRiskLevel(selectedTransaction.amountSC) or reuse the
existing expression) and replace branches that inspect selectedTransaction?.risk
with using riskStyles[riskLevel] for the modal’s className and any conditional
UI. Ensure all places that currently branch on selectedTransaction?.risk (the
detail modal rendering) use the unified riskLevel and the riskStyles constant so
list and modal styling are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e79bd332-591e-4fd0-8081-cb50fc48f574
📒 Files selected for processing (1)
dashboard/app/transactions/page.tsx
Use getRiskLevel(amountSC) for modal badge styling and label to match table behavior.
There was a problem hiding this comment.
Pull request overview
Fixes the duplicate close button on the transaction detail modal by removing the manual <Button>/<X> close control (the shadcn <DialogClose /> already provides one), and refactors the risk-level rendering to compute getRiskLevel once per row and look up styles from a static riskStyles map. The modal's risk badge is also switched from selectedTransaction?.risk to a derived selectedRiskLevel, unifying risk display logic between the table and the modal.
Changes:
- Remove redundant manual close button and unused
Xlucide-react import in the transaction detail modal. - Introduce
RiskLeveltype and module-scoperiskStylesmap; consume them in the table row Badge with a memoized per-rowriskLevel. - Use a derived
selectedRiskLevel(fromselectedTransaction.amountSC) for the modal's risk badge instead ofselectedTransaction?.risk.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addressed Issues:
Fixes #64
Screenshots/Recordings:
Before — Two overlapping x buttons on modal:
After — Single close button, modal behaves correctly.
Additional Notes:
The modal was rendering two overlapping
Xclose buttons:<DialogClose />(inside<DialogHeader>)<Button>with<X />from lucide-reactFix: Removed the manual
<Button onClick={() => setIsModalOpen(false)}>and the now-unusedXlucide import. The Dialog primitive handles close natively.Refactor — Badge Risk Level Logic
getRiskLevel(transaction.amountSC)was being called 3–4 times per table row inside inline ternary chains — redundant function calls on every render.Fix:
RiskLevelTypeScript type ("high" | "medium" | "low") and updatedgetRiskLevelreturn type accordinglyriskStylesRecord map at module scope (zero re-allocation per render).map(), computedconst riskLevel = getRiskLevel(transaction.amountSC)once per rowriskStyles[riskLevel]— clean, readable, and performantChecklist
AI Usage Disclosure
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Summary by CodeRabbit
Bug Fixes
Refactor