Skip to content

fix(ui): use DS::Pill for the background-jobs queue chip#2804

Open
rsnetworkinginc wants to merge 1 commit into
we-promise:mainfrom
rsnetworkinginc:fix/ds-pill-background-jobs-queue-chip
Open

fix(ui): use DS::Pill for the background-jobs queue chip#2804
rsnetworkinginc wants to merge 1 commit into
we-promise:mainfrom
rsnetworkinginc:fix/ds-pill-background-jobs-queue-chip

Conversation

@rsnetworkinginc

@rsnetworkinginc rsnetworkinginc commented Jul 25, 2026

Copy link
Copy Markdown

PR body — we-promise/sure — account: rsnetworkinginc (PR #2 on this repo)

Branch: fix/ds-pill-background-jobs-queue-chip (in WSL clone /root/sure, commit 58053fe4, based on origin/main @ 6b6ae0ca)
Title: fix(ds): render background-jobs queue chips with DS::Pill


Summary

Addresses Finding 1 of the DS Drift Patrol report in #2745.

The queue-status chips on the super-admin background jobs console (app/views/settings/background_jobs/show.html.erb:33, added in #2682) hand-roll the pill pattern with a raw rounded-full bg-container-inset px-3 py-1 span. That same PR's review cycle already fixed two other Drift Patrol comments before merge — this chip was the instance that slipped through.

What changed

  • The chip now renders through DS::Pill (badge mode — marker: false — with the :neutral semantic tone), replacing the hand-rolled span. One template, 1 chip block, no logic changes.
  • Text stays on the text-xs DS scale. The chip picks up the pill primitive's standard border/palette treatment (soft gray) instead of the raw bg-container-inset; the bespoke font-mono styling is dropped since the pill primitive owns typography — this is the intended DS conformance.

Test plan

  • bin/rails test — full suite green (5,920 runs, 23,808 assertions, 0 failures, 0 errors).
  • bin/rails test test/controllers/settings/background_jobs_controller_test.rb — green (renders this view with stubbed Sidekiq queues).
  • bundle exec erb_lint app/views/settings/background_jobs/show.html.erb — no offenses.
  • bin/rubocop — clean; bin/brakeman — no warnings.

Checklist for opening: base = main, allow edits from maintainers ON. Do NOT use closing keywords (#2745 has a second finding covered separately) — reference it as "Addresses Finding 1 of #2745".

Summary by CodeRabbit

  • Style
    • Updated background job queue indicators to use a consistent pill-style presentation.
    • Preserved queue names, sizes, and latency details while improving visual consistency.

The queue-status chips on the super-admin background jobs console used a
hand-rolled rounded-full/px/py span instead of the design-system pill
primitive. Render them through DS::Pill (badge mode, neutral tone) so the
chips pick up the DS palette, border, and text scale like every other
status badge.

Addresses Finding 1 of the DS Drift Patrol report in we-promise#2745.
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 515fc29d-9bce-4d34-a943-c9abf90526d7

📥 Commits

Reviewing files that changed from the base of the PR and between 6b6ae0c and 58053fe.

📒 Files selected for processing (1)
  • app/views/settings/background_jobs/show.html.erb

📝 Walkthrough

Walkthrough

The background jobs console now renders each queue entry with the reusable DS::Pill component, preserving queue name, size, and translated latency information while using a neutral tone without a marker.

Changes

Background jobs UI

Layer / File(s) Summary
Queue pill rendering
app/views/settings/background_jobs/show.html.erb
Queue entries now use DS::Pill with composed queue statistics, tone: :neutral, and marker: false.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related issues

  • we-promise/sure#2745 — Replaces the hand-rolled queue-status pill in the same background jobs view with DS::Pill.
  • we-promise/sure#2198 — Relates to replacing bespoke badge/pill markup with the reusable DS::Pill component.

Possibly related PRs

Suggested labels: contributor:verified, pr:verified

Suggested reviewers: gariasf, jjmata

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: replacing the background-jobs queue chip with DS::Pill.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

jjmata commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Reviewed the diff against app/components/DS/pill.rb/pill.html.erb and the surrounding conventions.

Component usage — correct. label, tone: :neutral, marker: false are all valid per the component's actual signature. marker: false is the right mode here (rounded-full, normal-case badge, dot-less by default), matching the original chip's look, and tone: :neutral resolves through SEMANTIC_TONE_ALIASES to the gray ramp — no raw hex/Tailwind colors introduced, in line with the functional-token convention.

Semantic meaning preserved — confirmed. The pre-PR chip never varied color by queue state (size/latency) — it was always the same flat bg-container-inset/text-secondary treatment regardless of backlog. So tone: :neutral doesn't lose any signal that existed before; nothing regresses here. (Optional, non-blocking idea for a follow-up: the codebase already has a precedent for state-driven tone on this same page's pattern — see app/views/settings/providers/_status_pill.html.erb, which maps ok/warn/errsuccess/warning/error. Could be worth doing the same for a backed-up/high-latency queue someday, but that's out of scope for a like-for-like component swap.)

i18n — no new user-facing strings; t(".stats.latency", value: queue[:latency]) is preserved unchanged (config/locales/views/settings/en.yml), and queue[:name]/queue[:size] are dynamic data, not translatable text, same as before.

One visual regression worth flagging: the original <span> had font-mono on the queue name/size/latency text (app/views/settings/background_jobs/show.html.erb:33, pre-PR). DS::Pill's container_classes has no monospace option, so this PR silently drops the monospace treatment for these numeric stats. It's cosmetic, but monospace was presumably chosen there to make the size/latency figures easier to scan at a glance across multiple queue pills — might be worth confirming that's an intentional trade-off rather than an oversight.

Everything else (params, tone semantics, i18n) looks clean.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants