fix: replace hand-rolled badges with DS::Pill in admin users view#1988
fix: replace hand-rolled badges with DS::Pill in admin users view#1988Kelvinchen03 wants to merge 1 commit into
Conversation
superagent-security
Bot
added
contributor:verified
📝 WalkthroughWalkthroughThis PR migrates inline badge markup in the admin users index view to the ChangesBadge Migration to DS::Pill
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90ae95a2fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <%= sub.active? ? "bg-success/10 text-success" : "bg-surface text-secondary" %>"> | ||
| <%= sub.status.humanize %> | ||
| </span> | ||
| <%= render DS::Pill.new(label: sub.status.humanize, tone: sub.active? ? :success : :neutral) %> |
There was a problem hiding this comment.
DS::Pill defaults to marker mode (marker: true) with a visible dot (show_dot: true), but these replacements are for plain badges that previously rendered normal-case text without a dot. As written, labels like subscription status and role names now render as marker chips (uppercase styling + dot), which is a visible regression from the prior UI and inconsistent with existing badge usages that pass marker: false (and often show_dot: false).
Useful? React with 👍 / 👎.
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/views/admin/users/index.html.erb`:
- Line 77: DS::Pill instances used for status/role badges are rendered with the
default marker=true causing marker/chip styling; update each render of DS::Pill
(e.g., render DS::Pill.new(label: sub.status.humanize, tone: sub.active? ?
:success : :neutral) and the similar role/status pill renders later) to pass
marker: false so they use badge mode (e.g., DS::Pill.new(..., marker: false));
keep other props (label, tone) unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9b2bb68-3ba9-4ea1-841a-1b1af93c205c
📒 Files selected for processing (1)
app/views/admin/users/index.html.erb
| <%= sub.active? ? "bg-success/10 text-success" : "bg-surface text-secondary" %>"> | ||
| <%= sub.status.humanize %> | ||
| </span> | ||
| <%= render DS::Pill.new(label: sub.status.humanize, tone: sub.active? ? :success : :neutral) %> |
There was a problem hiding this comment.
Use badge mode for these pills (marker: false) to avoid marker/chip styling regression.
At Line 77 and Lines 191-203, DS::Pill is rendered with defaults, which means marker: true (uppercase, rounded-md, sub-12px marker chrome). These are status/role badges and should use badge mode.
Proposed patch
-<%= render DS::Pill.new(label: sub.status.humanize, tone: sub.active? ? :success : :neutral) %>
+<%= render DS::Pill.new(label: sub.status.humanize, tone: sub.active? ? :success : :neutral, marker: false, size: :sm) %>
-<%= render DS::Pill.new(label: t(".roles.guest"), tone: :neutral) %>
+<%= render DS::Pill.new(label: t(".roles.guest"), tone: :neutral, marker: false, size: :sm) %>
-<%= render DS::Pill.new(label: t(".roles.member"), tone: :neutral) %>
+<%= render DS::Pill.new(label: t(".roles.member"), tone: :neutral, marker: false, size: :sm) %>
-<%= render DS::Pill.new(label: t(".roles.admin"), tone: :neutral) %>
+<%= render DS::Pill.new(label: t(".roles.admin"), tone: :neutral, marker: false, size: :sm) %>
-<%= render DS::Pill.new(label: t(".roles.super_admin"), tone: :success) %>
+<%= render DS::Pill.new(label: t(".roles.super_admin"), tone: :success, marker: false, size: :sm) %>Also applies to: 191-191, 195-195, 199-199, 203-203
🤖 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 `@app/views/admin/users/index.html.erb` at line 77, DS::Pill instances used for
status/role badges are rendered with the default marker=true causing marker/chip
styling; update each render of DS::Pill (e.g., render DS::Pill.new(label:
sub.status.humanize, tone: sub.active? ? :success : :neutral) and the similar
role/status pill renders later) to pass marker: false so they use badge mode
(e.g., DS::Pill.new(..., marker: false)); keep other props (label, tone)
unchanged.
|
Clean substitution. One small thing: the hand-rolled spans all had Generated by Claude Code |
2 participants
Summary
This PR fixes DS Drift Patrol violation #1978 by replacing 5 hand-rolled badge
<span>elements inapp/views/admin/users/index.html.erbwith the standardDS::Pillcomponent.Changes Made
bg-success/10 text-successDS::Pillwithtone: :successbg-surface text-secondaryDS::Pillwithtone: :neutralbg-surface text-primaryDS::Pillwithtone: :neutralbg-surface text-primaryDS::Pillwithtone: :neutralbg-surface text-primaryDS::Pillwithtone: :neutralbg-success/10 text-successDS::Pillwithtone: :successWhy This Change?
DS::PillcomponentRelated Issues
Fixes #1978
Type of Change
Checklist
Summary by CodeRabbit