Skip to content

fix: UI glitch: config is not visible#94325

Closed
sunlit-deng wants to merge 3 commits into
openclaw:mainfrom
sunlit-deng:fix/issue-94202
Closed

fix: UI glitch: config is not visible#94325
sunlit-deng wants to merge 3 commits into
openclaw:mainfrom
sunlit-deng:fix/issue-94202

Conversation

@sunlit-deng

@sunlit-deng sunlit-deng commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Raw config JSON textarea was invisible when switching from Form to Raw mode in Settings.
  • Root cause: .config-content container retained scroll position from Form mode, so the textarea rendered above the scrolled viewport.
  • Fix: detect form-mode transitions in renderConfig() and reset .config-content scroll to top.

Fixes #94202

Real behavior proof (required for external PRs)

Behavior addressed: Raw config textarea is visible immediately after switching from Form to Raw mode.

Real setup tested:

  • Runtime: Node.js v24.13.1, OpenClaw gateway dev (port 18789, token auth)
  • Browser: Playwright Chromium (headless, 1280x720)
  • Language: English

Exact steps or command run after fix:

# Start gateway
cd openclaw && pnpm gateway:watch &
sleep 20
curl -s --noproxy '*' http://127.0.0.1:18789/health

# Playwright verification
npx playwright-cli open "http://127.0.0.1:18789/chat?token=<token>&lang=en"
# Settings -> Config -> Advanced -> Raw -> Reveal -> screenshot

After-fix evidence:

1-form-mode

2-raw-mode

textareaVisible: true
boundingBox:  {x:301, y:273.8, width:936, height:500}
viewport:      1280x720

{"ok":true,"status":"live"}

Observed result after the fix: Raw config textarea renders in the visible viewport (y=273.8 in 720px-high window). No manual scroll needed after switching from Form to Raw mode.

What was not tested: Other settings sections — same .config-content container, scroll reset applies uniformly. Non-Chromium browsers not explicitly tested but the scroll API is standard.

Tests and validation

  • node scripts/run-vitest.mjs run ui/src/ui/views/config.browser.test.ts — 26/26 pass
  • "switches mode via the sidebar toggle" — Raw button click exercised
  • "keeps sensitive raw config hidden until reveal" — textarea rendering validated

Change details

ui/src/ui/views/config.ts (+21/-32 lines):

// Scroll helper: target-based (nav clicks) with global fallback (form/raw toggle)
const resetContentScroll = (target: EventTarget | null) => {
  queueMicrotask(() => {
    const origin = target instanceof Element ? target : null;
    const content = origin
      ?.closest(".config-main")
      ?.querySelector<HTMLElement>(".config-content")
      ?? globalThis.document?.querySelector<HTMLElement>(".config-content");
    if (!content) return;
    if (typeof content.scrollTo === "function") {
      content.scrollTo({ top: 0, left: 0, behavior: "auto" });
      return;
    }
    content.scrollTop = 0;
    content.scrollLeft = 0;
  });
};

// In renderConfig():
if (lastFormModeForScroll !== null && lastFormModeForScroll !== formMode) {
  resetContentScroll(null);
}
lastFormModeForScroll = formMode;

Before/after data flow:

Before: User clicks "Raw" -> .config-content.scrollTop = Form-mode position
        -> textarea above viewport -> appears empty

After:  User clicks "Raw" -> mode change detected
        -> queueMicrotask(resetContentScroll)
        -> .config-content.scrollTo({top:0,left:0}) -> textarea visible

Risk checklist

Did user-visible behavior change? (No) — Scroll-position reset only.

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No)

What is the highest-risk area?

  • resetContentScroll uses globalThis.document?.querySelector. In SSR contexts without document, the ?? fallback returns early (safe no-op).

How is that risk mitigated?

  • Optional chaining returns undefined when document absent. lastFormModeForScroll !== null guard prevents spurious scroll on initial render.

Current review state

What is the next action?

  • Maintainer review

What is still waiting on author, maintainer, CI, or external proof?

  • CI checks

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 11:08 PM ET / 03:08 UTC.

Summary
The PR tracks effective Form/Raw mode changes in renderConfig() and resets the Settings .config-content scroll position when the mode changes.

PR surface: Source +8. Total +8 across 1 file.

Reproducibility: yes. at source level. Current main resets .config-content for section navigation but not for Form/Raw toggles, which makes the retained-scroll failure path credible without a fresh live browser repro.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94202
Summary: This PR is a candidate fix for the canonical Raw config visibility issue; sibling PRs target the same symptom with different approaches.

Members:

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

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] Add a focused browser test that sets .config-content.scrollTop, switches Form to Raw, and asserts the reset if maintainers want test-backed coverage before merge.

Risk before merge

  • [P1] Three open closing PRs target [Bug]: UI glitch: config is not visible #94202 with different root-cause theories, so maintainers should avoid merging more than one.
  • [P1] This branch has inspected screenshot proof but no focused regression test that scrolls .config-content, switches Form to Raw, and asserts the reset.

Maintainer options:

  1. Decide the mitigation before merge
    Land one canonical Control UI fix that resets the Raw-mode scroller while preserving sensitive-value reveal behavior, with focused regression coverage if maintainers require it.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] Maintainers need to pick the canonical linked fix; there is no narrow automated repair blocker on this branch.

Security
Cleared: Cleared: the diff only changes local Control UI scroll behavior and does not alter secret rendering, dependencies, workflows, package metadata, or network execution paths.

Review details

Best possible solution:

Land one canonical Control UI fix that resets the Raw-mode scroller while preserving sensitive-value reveal behavior, with focused regression coverage if maintainers require it.

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

Yes, at source level. Current main resets .config-content for section navigation but not for Form/Raw toggles, which makes the retained-scroll failure path credible without a fresh live browser repro.

Is this the best way to solve the issue?

Yes. This is the best-supported current candidate because it targets the retained .config-content scroll position while preserving Raw config redaction, unlike the open reveal-bypass sibling and the less-targeted CSS overflow sibling.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This PR addresses a normal-priority Settings UI visibility bug with limited blast radius.
  • 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 supplied screenshots directly show the after-fix Raw config textarea visible in a real browser Settings flow, with redacted values and bounding-box evidence in the PR body.
  • proof: sufficient: Contributor real behavior proof is sufficient. The supplied screenshots directly show the after-fix Raw config textarea visible in a real browser Settings flow, with redacted values and bounding-box evidence in the PR body.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The supplied screenshots directly show the after-fix Raw config textarea visible in a real browser Settings flow, with redacted values and bounding-box evidence in the PR body.
Evidence reviewed

PR surface:

Source +8. Total +8 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 26 18 +8
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 26 18 +8

What I checked:

  • Repository policy read: Read the full root AGENTS.md and UI-scoped guide; the review applied OpenClaw's broad PR review, proof, linked-PR, and UI-scope requirements. (AGENTS.md:1, a1828110704f)
  • Current main behavior: Current main has a scroll reset helper for section navigation, but the helper only resolves from an event target inside .config-main. (ui/src/ui/views/config.ts:1304, a1828110704f)
  • Current Form/Raw toggle path: The Form and Raw buttons currently call onFormModeChange() directly, so a retained .config-content scroll position is not reset by the mode switch on current main. (ui/src/ui/views/config.ts:1461, a1828110704f)
  • PR implementation: The PR adds lastFormModeForScroll and calls a document-fallback scroll reset when effective Form/Raw mode changes. (ui/src/ui/views/config.ts:1238, 0b47dc113822)
  • Sensitive raw config behavior preserved: Current raw mode still computes blurred from sensitive values and reveal state, and this PR does not change that secret-reveal branch. (ui/src/ui/views/config.ts:1844, a1828110704f)
  • Adjacent tests: Existing browser tests cover section-navigation scroll reset and hidden-until-reveal behavior for sensitive raw config, but not the exact Form-to-Raw retained-scroll reset. (ui/src/ui/views/config.browser.test.ts:457, a1828110704f)

Likely related people:

  • BunsDev: Commit messages credit BunsDev with the Settings overhaul and the existing settings scroll-reset path that this PR extends. (role: adjacent feature owner; confidence: high; commits: a45ebf3281ed, 2cfb660a9bb8; files: ui/src/ui/views/config.ts, ui/src/styles/config.css, ui/src/ui/views/config.browser.test.ts)
  • steipete: Authored the raw-config reveal-to-edit flow that defines the sensitive-value behavior this PR preserves. (role: recent area contributor; confidence: high; commits: f0c1057f68b6; files: ui/src/ui/views/config.ts, ui/src/ui/views/config.browser.test.ts)
  • Shakker: Recent browser-test history covers raw config secret masking adjacent to this bug's Raw-mode behavior. (role: recent test contributor; confidence: medium; commits: 16f2b22b331c; files: ui/src/ui/views/config.browser.test.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.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: supplied External PR includes structured after-fix real behavior proof. and removed proof: supplied External PR includes structured after-fix real behavior proof. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026
@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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. P2 Normal backlog priority with limited blast radius. labels Jun 18, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026
This was referenced Jun 18, 2026
Closed
Closed
Closed
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. and removed 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. labels Jun 19, 2026
@Takhoffman

Takhoffman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper automerge is enabled.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

  • 2026-06-23 13:59:46 UTC review queued 0b47dc113822 (queued)
  • 2026-06-23 15:51:18 UTC review queued 0b47dc113822 (queued)

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label Jun 23, 2026
@clawsweeper clawsweeper Bot added the clawsweeper:human-review Needs maintainer review before ClawSweeper can continue label Jun 23, 2026
@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper is pausing this repair loop for human review.

Source: clawsweeper[bot]
Reason: - [P2] Maintainers need to pick the canonical linked fix; there is no narrow automated repair blocker on this branch.; Cleared: Cleared: the diff only changes local Control UI scroll behavior and does not alter secret rendering, dependencies, workflows, package metadata, or network execution paths. (sha=0b47dc1138228b7cd5df5fdc8f775aa0a36ccad2)

Why human review is needed:
This item has security-sensitive risk. ClawSweeper is pausing instead of making an autonomous change that could affect trust, credentials, permissions, or exposure.

What the maintainer can do as a next step:
If the maintainer accepts the current risk and wants ClawSweeper to continue merge gates, comment @clawsweeper approve. If the security-sensitive detail still needs changes, describe the safe path or push the fix, then comment @clawsweeper automerge. If the risk should not be automated, keep the PR paused for manual review or comment @clawsweeper stop.

I added clawsweeper:human-review and left the final call with a maintainer.

@Takhoffman

Takhoffman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper approve

@clawsweeper clawsweeper Bot removed the clawsweeper:human-review Needs maintainer review before ClawSweeper can continue label Jun 23, 2026
@Takhoffman

Takhoffman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper clawsweeper Bot mentioned this pull request Jun 23, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the contribution here. ClawSweeper tried the original lane first, but branch permissions blocked the push, so a replacement PR is carrying the fix forward.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
Replacement PR: #96145
Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place.
This source PR is being closed only under the explicit source-close setting for this ClawSweeper run.
The replacement PR carries the original credit trail forward.
Co-author credit kept:

fish notes: reasoning high; reviewed against a6ea91e.

@clawsweeper clawsweeper Bot closed this Jun 23, 2026
@sunlit-deng

sunlit-deng commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

ClawSweeper 🐠 reef update

Thanks for the contribution here. ClawSweeper tried the original lane first, but branch permissions blocked the push, so a replacement PR is carrying the fix forward.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch. Replacement PR: #96145 Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place. This source PR is being closed only under the explicit source-close setting for this ClawSweeper run. The replacement PR carries the original credit trail forward. Co-author credit kept:

fish notes: reasoning high; reviewed against a6ea91e.

Permissions have been updated, you can push to my original branch now.

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 clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS 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.

[Bug]: UI glitch: config is not visible

2 participants

@sunlit-deng @Takhoffman