Skip to content

fix(ui): give roster rows their avatar-text gap and add presence mock fixtures#111689

Merged
steipete merged 2 commits into
mainfrom
steipete/roster-menu-spacing-fixtures
Jul 20, 2026
Merged

fix(ui): give roster rows their avatar-text gap and add presence mock fixtures#111689
steipete merged 2 commits into
mainfrom
steipete/roster-menu-spacing-fixtures

Conversation

@steipete

@steipete steipete commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Follow-up to #111668: the redesigned who's-online roster rendered with no gap between the avatar and the name/email text, so rows read as cramped and unfinished. Root cause: Web Awesome's dropdown-item spaces its icon slot via #icon ::slotted(*) { margin-inline-end: 0.75em }, but the slotted openclaw-viewer-avatar host is display: contents — it generates no box, so the margin silently never applies.

Separately, the mock harness could not exercise any presence surface: its system-presence fixtures carry no user identities, so the footer facepile and roster only rendered against a real gateway (the #111668 screenshots needed manual property injection).

Why This Change Was Made

  • ui/src/styles/layout.css: the roster row now owns its avatar-to-text gap (margin-inline-end: 10px on the light-DOM .viewer-avatar, where a real box exists), with a comment explaining why the slot margin can't do it; row padding bumped 5px → 6px vertical.
  • ui/src/test-helpers/control-ui-e2e.ts: new optional scenario field presenceUsers — the shared mock gateway now emits a presence list in the connect snapshot, with the self-flagged entry adopting the connecting client's instanceId so "you" resolves. Empty by default; existing scenarios and tests are unaffected.
  • scripts/control-ui-mock-dev.ts: the dev harness ships three presence users (self + named + email-only), lighting up the footer identity, facepile, and roster from fixtures alone — no injection needed to develop or verify presence UI.

User Impact

Roster rows get proper breathing room between avatar and text in both themes. No runtime behavior change outside CSS; the rest is test/dev tooling.

Evidence

Fixture-driven captures (no property injection) from scripts/control-ui-mock-dev.ts:

Dark Light
dark light
  • node scripts/run-vitest.mjs ui/src/test-helpers/control-ui-e2e.mock-gateway.test.ts ui/src/test-helpers/control-ui-e2e.test.ts ui/src/components/viewer-facepile.test.ts — 14/14 passed.
  • oxfmt --check clean on all three files; git diff --check clean.
  • Codex autoreview (gpt-5.6-sol, branch vs origin/main) — clean, no findings.
  • Live-verified: mock dev server renders footer facepile + roster from the new fixtures (screenshots above).

@steipete
steipete marked this pull request as ready for review July 20, 2026 06:00
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui scripts Repository scripts size: S maintainer Maintainer-authored PR labels Jul 20, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 20, 2026
@clawsweeper

clawsweeper Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 2:03 AM ET / 06:03 UTC.

Summary
Adds explicit avatar-to-text spacing for Control UI presence-roster rows and lets the shared mock gateway/dev scenario provide presence users.

PR surface: Source +44, Other +7. Total +51 across 3 files.

Reproducibility: no. high-confidence independent reproduction run was completed in this read-only review. The supplied source explanation identifies the display: contents margin behavior and the PR includes fixture-driven before/after UI evidence.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Refresh the branch against current main and confirm the final exact-head checks before merge.

Risk before merge

  • [P1] The branch is reported as behind main; before merge, a maintainer should refresh the three-way result and let the remaining exact-head checks finish.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the rendered-element spacing fix and reusable mock presence fixtures, then land only after a maintainer confirms the refreshed merge result on current main.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The protected maintainer label and behind merge base require human review of the final merge result; no narrow automated repair is identified.

Security
Cleared: The supplied diff is limited to CSS and local Control UI mock/dev fixture code, with no dependency, workflow, permission, secret, or artifact-execution change.

Review details

Best possible solution:

Keep the rendered-element spacing fix and reusable mock presence fixtures, then land only after a maintainer confirms the refreshed merge result on current main.

Do we have a high-confidence way to reproduce the issue?

No high-confidence independent reproduction run was completed in this read-only review. The supplied source explanation identifies the display: contents margin behavior and the PR includes fixture-driven before/after UI evidence.

Is this the best way to solve the issue?

Yes, based on the supplied diff: applying spacing to the rendered .viewer-avatar avoids the ineffective slot-host margin, while optional snapshot fixtures exercise the UI without changing production presence behavior.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 8c9ecd924238.

Label changes

Label changes:

  • add P3: This is a small Control UI polish and developer-fixture improvement without evidence of message, session, auth, security, or availability impact.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.

Label justifications:

  • P3: This is a small Control UI polish and developer-fixture improvement without evidence of message, session, auth, security, or availability impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body provides direct dark/light fixture-driven roster captures plus focused test results; redact any private values if refreshed evidence is posted.
Evidence reviewed

PR surface:

Source +44, Other +7. Total +51 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 46 2 +44
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 7 0 +7
Total 3 53 2 +51

What I checked:

Likely related people:

  • steipete: Authored the earlier merged Control UI roster redesign and this narrowly scoped follow-up on the same roster styling and fixture path. (role: recent area contributor; confidence: medium; commits: a22ede72e494, 094b76d3a58b; files: ui/src/styles/layout.css, ui/src/test-helpers/control-ui-e2e.ts, scripts/control-ui-mock-dev.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete

steipete commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Re the rank-up move (refresh against current main): skipping the rebase per repo policy — merge drift is advisory unless strict drift is enabled, and GitHub reports the branch MERGEABLE with no conflicts. scripts/pr merge-verify re-evaluates drift relevance at merge time, and the drifted mainline files do not touch this PR's surface (roster CSS, mock-gateway test helper, mock-dev fixtures). Exact-head CI is green on 094b76d.

@steipete
steipete merged commit e2e394e into main Jul 20, 2026
203 of 215 checks passed
@steipete
steipete deleted the steipete/roster-menu-spacing-fixtures branch July 20, 2026 06:38
@steipete

steipete commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 20, 2026
@steipete
fix(ui): give roster rows their avatar-text gap and add presence mock…
4400524 
… fixtures (openclaw#111689)

* fix(ui): add avatar-text gap in roster menu and presence mock fixtures

* refactor(ui): keep presence snapshot helper out of the connect auth hunk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: web-ui App: web-ui maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete