Replace text actions with icons on domain settings page (Fixes #405) #418
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughReplaces text-based Edit/Delete buttons with icon-only buttons wrapped in tooltips in the domain settings DomainActions component. Keeps existing handlers and modal flow unchanged. Imports lucide-react icons (Edit, Trash2) and Tooltip components. Adjusts button styling to small icon buttons while preserving component signature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DomainActions
participant Tooltip
participant UpdateDomainDialog as EditDialog
participant DeleteDomain as DeleteDialog
User->>DomainActions: Hover Edit icon
DomainActions->>Tooltip: Show "Edit" label
User->>DomainActions: Click Edit icon
DomainActions->>EditDialog: Open edit modal
User->>DomainActions: Hover Delete icon
DomainActions->>Tooltip: Show "Delete" label
User->>DomainActions: Click Delete icon
DomainActions->>DeleteDialog: Open delete modal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![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)
view/app/settings/domains/components/domain-actions.tsx (2)
31-45: Add accessible name and prevent unintended form submit.Icon-only buttons need an accessible name; tooltips don’t provide one to AT. Also explicitly set type="button".
Apply this diff:
<TooltipTrigger asChild> <Button variant="ghost" size="sm" className="h-8 w-8 p-0 text-primary hover:text-primary/80 hover:bg-primary/10" - onClick={handleEdit} + onClick={handleEdit} + type="button" + aria-label={t('settings.domains.actions.edit')} + title={t('settings.domains.actions.edit')} > - <Edit className="h-4 w-4" /> + <Edit aria-hidden="true" className="h-4 w-4" /> </Button>
33-36: Optional: increase hit area on small screens.Consider 40–44px targets for touch. Example: make default h/w 10 (40px) and reduce on md+.
- className="h-8 w-8 p-0 text-primary hover:text-primary/80 hover:bg-primary/10" + className="h-10 w-10 md:h-8 md:w-8 p-0 text-primary hover:text-primary/80 hover:bg-primary/10"- className="h-8 w-8 p-0 text-destructive hover:text-destructive/90 hover:bg-destructive/10" + className="h-10 w-10 md:h-8 md:w-8 p-0 text-destructive hover:text-destructive/90 hover:bg-destructive/10"Also applies to: 51-54
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
view/app/settings/domains/components/domain-actions.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
view/app/settings/domains/components/domain-actions.tsx (1)
view/components/rbac/PermissionGuard.tsx (1)
ResourceGuard(78-95)
🔇 Additional comments (2)
view/app/settings/domains/components/domain-actions.tsx (2)
3-4: LGTM on imports.Correctly pulls Tooltip bits and lucide icons per issue #405.
48-62: Use semantic destructive tokens + add a11y props (and type).Destructive tokens exist in the repo (view/components/colors.ts, view/components/ui/button.tsx, view/app/globals.css) — replace the hard-coded red classes and add type/ARIA as shown below.
<TooltipTrigger asChild> <Button variant="ghost" size="sm" - className="h-8 w-8 p-0 text-red-500 hover:text-red-600 hover:bg-red-50" + className="h-8 w-8 p-0 text-destructive hover:text-destructive/90 hover:bg-destructive/10" onClick={handleDelete} + type="button" + aria-label={t('settings.domains.actions.delete')} + title={t('settings.domains.actions.delete')} > - <Trash2 className="h-4 w-4" /> + <Trash2 aria-hidden="true" className="h-4 w-4" /> </Button>Confirm there are no visible text-labeled "Edit"/"Delete" Buttons under view/app/settings/domains — the prior grep for those labels failed with "unrecognized file type: tsx", so re-run the search without a file-type filter or verify manually.
What does this PR do?
Why?
Related issue
Fixes #405
#405
nixopus_issues_405.mp4
Summary by CodeRabbit