Skip to content

fix: align talk settings native select rows#97346

Closed
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:javis/96915-remove-talk-select-label
Closed

fix: align talk settings native select rows#97346
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:javis/96915-remove-talk-select-label

Conversation

@arkyu2077

Copy link
Copy Markdown
Contributor

Summary\n- remove the extra native select value label rendered inside Talk settings rows\n- keep the select itself as the single visible value source so Voice and Sensitivity align with the Model row\n- update the Talk settings test to validate native select values/options instead of the removed duplicate label span\n\n## Testing\n- node scripts/run-vitest.mjs ui/src/ui/views/chat.test.ts\n\nCloses #96915

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 28, 2026
@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close: the useful work is superseded by #96925, which fixes the same Talk settings selected-label root cause more completely and has live mergeable/proof-positive evidence.

Root-cause cluster
Relationship: superseded
Canonical: #96925
Summary: This PR is a duplicate candidate fix for the same Talk settings selected-label alignment root cause, and the sibling PR is the stronger canonical landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Land #96925 as the canonical source/test fix, then close the duplicate candidate branches and the linked issue after merge.

So I’m closing this here and keeping the remaining discussion on #96925.

Review details

Best possible solution:

Land #96925 as the canonical source/test fix, then close the duplicate candidate branches and the linked issue after merge.

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

Yes. Current main source renders Voice and Sensitivity with an extra selected-label span before the select while Model has only a label and input, matching the reporter screenshot and sibling proof.

Is this the best way to solve the issue?

No for this PR as the best landing path. The source fix idea is valid, but #96925 removes the same root cause more completely and already has inspected screenshot proof.

Security review:

Security review cleared: The diff only changes Control UI rendering, CSS, and focused tests; it does not touch dependencies, CI, secrets, permissions, package metadata, or code-execution surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current main source reproduces the issue: Current main renders an extra .agent-chat__talk-select-label span inside renderNativeTalkSelect, while the adjacent Model control renders only label plus input in the same primary row. (ui/src/ui/views/chat.ts:288, 69af58ba2613)
  • Current main layout makes the mismatch visible: The primary Talk settings controls are a three-column grid and each field is a flex column with a 6px gap, so the extra selected-label row shifts select controls lower than the Model input. (ui/src/styles/chat/layout.css:1013, 69af58ba2613)
  • This PR is a partial duplicate fix: The patch removes the visible selected-label rendering and CSS, but leaves the selectedLabel parameter and Sensitivity call-site plumbing in place. (ui/src/ui/views/chat.ts:292, 436127344ae5)
  • Canonical sibling patch is cleaner: The sibling PR removes the same selected-label rendering plus the selectedLabel parameter, Sensitivity label calculation, and call-site argument, with adjacent test updates. (ui/src/ui/views/chat.ts:290, 7d47d0f5d6a9)
  • Canonical sibling is viable: Live metadata shows fix(ui): remove redundant selectedLabel rendering in Talk settings (#… #96925 is open, not draft, MERGEABLE/CLEAN, and labeled proof: sufficient plus proof: 📸 screenshot. (7d47d0f5d6a9)
  • Canonical proof inspected: The sibling PR's downloaded 1576x211 PNG shows Voice, Model, and Sensitivity aligned in the Talk settings row after the source fix. (7d47d0f5d6a9)

Likely related people:

  • vincentkoc: GitHub and local commit metadata tie a821ea110bbe to preserving Talk select labels in the affected shared helper. (role: feature introducer; confidence: high; commits: a821ea110bbe; files: ui/src/ui/views/chat.ts)
  • alexzhu0: GitHub commit metadata ties 85e5d486df11 to the Control UI chat render refactor that carried this Talk settings path forward. (role: affected refactor author; confidence: high; commits: 85e5d486df11; files: ui/src/ui/views/chat.ts, ui/src/styles/chat/layout.css, ui/src/ui/views/chat.test.ts)
  • shushushv: GitHub commit metadata ties d211d49011ba to realtime Talk provider and transport option work in the same chat view and adjacent tests. (role: recent adjacent contributor; confidence: medium; commits: d211d49011ba; files: ui/src/ui/views/chat.ts, ui/src/ui/views/chat.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 69af58ba2613.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 28, 2026
@steipete steipete closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants