Skip to content

improve(ui): remove the Roundness appearance setting#102641

Merged
steipete merged 1 commit into
mainfrom
claude/charming-lumiere-282269
Jul 9, 2026
Merged

improve(ui): remove the Roundness appearance setting#102641
steipete merged 1 commit into
mainfrom
claude/charming-lumiere-282269

Conversation

@steipete

@steipete steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Related: #49436

What Problem This Solves

The Control UI exposed a browser-local "Roundness" appearance setting (segmented rows in Quick Settings and the Appearance page) that scaled every --radius-* CSS variable through a global multiplier. Corner radius is a theme design decision (Claw/Knot/Dash and imported custom themes); a global user knob distorts theme intent. The multiplier also only took effect after changing an appearance setting in-session — it was never applied at startup — so a saved non-default roundness silently reverted on every page load anyway.

Why This Change Was Made

Removes the knob end-to-end so corner radius is always theme-owned: the Roundness rows in Quick Settings and the Appearance page, the borderRadius field/normalization/persistence in UI settings, applyBorderRadius() and its BASE_RADII scaling table, the prop threading through the config views, the orphaned .settings-roundness styles, and the raw-copy i18n baseline entries for the removed labels. The --radius-* variables stay as static theme values in ui/src/styles/base.css. Stale borderRadius localStorage keys are ignored and dropped on the next settings write; no compat shim for browser-local state that was never applied at boot.

User Impact

The Roundness rows disappear from Quick Settings and the Appearance settings page. Every theme renders with its standard corner radii, matching what users already saw on each fresh page load. The Text size (accessibility) setting is untouched.

Evidence

  • pnpm tsgo:core and pnpm tsgo:test:ui: clean
  • node scripts/run-vitest.mjs on the six touched test files: 214 tests passed (187 node/jsdom + 27 browser)
  • pnpm ui:i18n:check: green after node --import tsx scripts/control-ui-i18n.ts sync --write; baseline diff is removal-only (56 lines for the dropped raw strings)
  • oxfmt --check and scripts/run-oxlint.mjs on touched files: clean
  • Codex autoreview (local diff, thinking=high): clean, no accepted findings

Release-note context: Control UI/appearance: remove the Roundness setting; corner radius is now owned by themes.

Corner radius is owned by themes; drop the browser-local multiplier that
scaled all --radius-* variables (added in #49436). Stale borderRadius
localStorage keys are ignored and dropped on next settings write.
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: M maintainer Maintainer-authored PR labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 6:14 AM ET / 10:14 UTC.

Summary
The PR removes the Control UI Roundness appearance control, its borderRadius settings/persistence path, related prop threading, styles, i18n baseline entries, and test fixtures.

PR surface: Source -225, Tests -22. Total -247 across 12 files.

Reproducibility: not applicable. as a cleanup PR rather than a bug report. Source inspection of current main confirms the old multiplier was only invoked by in-session applySettings, not by startup settings load.

Review metrics: 1 noteworthy metric.

  • Persisted UI preference removed: 1 removed (borderRadius). The PR removes a browser-local appearance field from UiSettings and the persisted payload, so maintainers should notice the upgrade behavior before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
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] Maintainers should explicitly accept the Roundness preference removal and its upgrade impact before merge.

Risk before merge

  • [P1] Merging intentionally removes a shipped browser-local borderRadius appearance preference; users who relied on changing roundness during a session lose that knob, even though current main did not reapply it on fresh startup.

Maintainer options:

  1. Accept the preference removal (recommended)
    Merge as-is if maintainers agree the startup-broken browser-local Roundness preference should be retired in favor of theme-owned radii.
  2. Add a transition path
    If preserving user choice matters, keep the preference or add a clear migration/UX note before dropping borderRadius from persisted settings.

Next step before merge

  • [P1] Protected maintainer-labeled PR removes a persisted UI preference; no mechanical repair is indicated, but merge should wait for maintainer acceptance of the preference-removal impact and normal merge gates.

Maintainer decision needed

  • Question: Should OpenClaw retire the shipped browser-local Roundness preference now and make corner radius solely theme-owned?
  • Rationale: The code removal is clean, but deleting a persisted user-visible appearance setting is a product/compatibility choice rather than a purely mechanical cleanup.
  • Likely owner: steipete — This handle authored the current PR and recently carried the settings UI surface through the full-page settings redesign.
  • Options:
    • Accept theme-owned radius (recommended): Merge the current removal and own the visual-preference reset because the old setting was not applied on fresh startup.
    • Preserve or transition the preference: Keep the setting or add a visible transition path if maintainers want existing users to retain explicit roundness control.

Security
Cleared: The diff only removes Control UI settings/rendering/CSS/i18n/test code and does not touch dependencies, workflows, secrets, network calls, package metadata, or supply-chain execution paths.

Review details

Best possible solution:

Land this only if maintainers explicitly want radius to be theme-owned and accept dropping stale borderRadius; otherwise preserve the setting or provide a clearer transition path.

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

Not applicable as a cleanup PR rather than a bug report. Source inspection of current main confirms the old multiplier was only invoked by in-session applySettings, not by startup settings load.

Is this the best way to solve the issue?

Yes, the implementation is the narrow maintainable removal: it deletes the UI rows, prop threading, normalization/persistence, and orphaned CSS together. The remaining question is product acceptance of removing the shipped preference.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P3: This is low-risk Control UI cleanup/polish with limited blast radius and no evidence of runtime, security, channel, or data-loss impact.
  • add merge-risk: 🚨 compatibility: The diff removes the persisted borderRadius appearance preference and drops stale stored values on future settings writes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is maintainer-labeled, so the external contributor proof gate does not apply; the body provides test evidence but no separate live UI screenshot or recording.

Label justifications:

  • P3: This is low-risk Control UI cleanup/polish with limited blast radius and no evidence of runtime, security, channel, or data-loss impact.
  • merge-risk: 🚨 compatibility: The diff removes the persisted borderRadius appearance preference and drops stale stored values on future settings writes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is maintainer-labeled, so the external contributor proof gate does not apply; the body provides test evidence but no separate live UI screenshot or recording.
Evidence reviewed

PR surface:

Source -225, Tests -22. Total -247 across 12 files.

View PR surface stats
Area Files Added Removed Net
Source 6 2 227 -225
Tests 6 0 22 -22
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 12 2 249 -247

What I checked:

  • Live PR state: Live GitHub data shows this PR is open, non-draft, labeled maintainer, touches 12 UI/settings files, and is currently behind the base while still reported mergeable. (7b0829f6536d)
  • Current-main startup behavior: On current main, applyBorderRadius exists and ConfigPage.connectedCallback reloads settings, but the radius multiplier is only called from applySettings, supporting the PR claim that persisted roundness was not applied on fresh startup. (ui/src/pages/config/config-page.ts:266, fc5a4defaf62)
  • PR removes settings field and persistence: At the PR head, UiSettings no longer exposes borderRadius, defaults/load normalization no longer read it, and the persisted settings object omits it so stale stored keys drop on the next write. (ui/src/app/settings.ts:85, 7b0829f6536d)
  • PR removes runtime multiplier path: At the PR head, ConfigPage.applySettings persists theme/custom-theme/text-scale only, applies text scale, and refreshes the theme without any applyBorderRadius call or setBorderRadius method. (ui/src/pages/config/config-page.ts:517, 7b0829f6536d)
  • PR removes the visible controls: Quick Settings now exposes theme mode and text size without a Roundness row, and the Appearance page proceeds from theme import to Text size with no Roundness section. (ui/src/pages/config/quick.ts:88, 7b0829f6536d)
  • No leftover setting symbols: A repository grep at the PR head found no remaining Roundness/setting symbols; remaining borderRadius hits are unrelated CSS geometry assertions in tests. (7b0829f6536d)

Likely related people:

  • steipete: Authored and merged the settings takeover that carried the Roundness setting into the current ui/src/app and ui/src/pages/config files, and CODEOWNERS names this handle for ownership-rule changes though not for UI specifically. (role: recent area contributor; confidence: high; commits: 9269a27f18c3, 7b0829f6536d; files: ui/src/app/settings.ts, ui/src/pages/config/config-page.ts, ui/src/pages/config/quick.ts)
  • BunsDev: Authored the merged appearance PR that introduced the Roundness slider and borderRadius preference before the later settings-file reshuffle. (role: feature introducer; confidence: high; commits: 7fe4a3254b8e, df72ca1ecec7; files: ui/src/ui/app-settings.ts, ui/src/ui/app.ts, ui/src/ui/views/config.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.

@clawsweeper clawsweeper Bot added 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. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 9, 2026
@steipete
steipete merged commit 06b33f5 into main Jul 9, 2026
175 of 186 checks passed
@steipete
steipete deleted the claude/charming-lumiere-282269 branch July 9, 2026 10:20
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
Corner radius is owned by themes; drop the browser-local multiplier that
scaled all --radius-* variables (added in openclaw#49436). Stale borderRadius
localStorage keys are ignored and dropped on next settings write.
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 maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant