Skip to content

fix(ui): add guarded dashboard shortcuts#81958

Closed
hclsys wants to merge 1 commit into
openclaw:mainfrom
hclsys:fix/dashboard-shortcuts-81946
Closed

fix(ui): add guarded dashboard shortcuts#81958
hclsys wants to merge 1 commit into
openclaw:mainfrom
hclsys:fix/dashboard-shortcuts-81946

Conversation

@hclsys

@hclsys hclsys commented May 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Fixes #81946

Summary

  • add a guarded document-level dispatcher for dashboard chat shortcuts
  • / moves focus to the chat composer, switching to Chat first when needed
  • N triggers the same new-message jump path only when the affordance is active
  • Esc dismisses palette/mobile controls/session notice/focus mode without stealing editable-field keys
  • treat contenteditable="plaintext-only" targets as editable so the dispatcher does not intercept editor input

Real behavior proof

  • Behavior or issue addressed: Control UI dashboard shortcuts now handle /, N, and Esc through a guarded document-level dispatcher while ignoring editable text targets, including contenteditable="plaintext-only".

  • Real environment tested: Local OpenClaw Control UI Vite dev server from this checkout, loaded in Chromium at http://127.0.0.1:5174/overview.

  • Exact steps or command run after this patch:

    1. Ran pnpm --dir ui dev --host 127.0.0.1 --port 5174.
    2. Opened http://127.0.0.1:5174/overview in Chromium headless shell via Playwright.
    3. In the live page, set <openclaw-app> to connected, dispatched /, N, and Escape from document scope, the chat composer textarea, and a focused contenteditable="plaintext-only" element.

  • Evidence after fix: Browser console output from the live Control UI page:

    {
  "slash": {
    "beforeTab": "overview",
    "defaultPrevented": true,
    "afterTab": "chat",
    "hasComposer": true,
    "focusedComposer": true
  },
  "newMessagesShortcut": {
    "jumpDefaultPrevented": true,
    "editableJumpDefaultPrevented": false,
    "scrollCalls": 1
  },
  "escape": {
    "escapeDefaultPrevented": true,
    "paletteAfterEscape": false,
    "editableEscapeDefaultPrevented": false,
    "mobileAfterEditableEscape": true
  },
  "plaintextOnly": {
    "slash": false,
    "n": false,
    "escape": false,
    "paletteOpen": true,
    "mobileOpen": true,
    "scrollCalls": 1
  }
}

  • Observed result after fix: / routed the real Control UI shell from Overview to Chat and focused the composer; N invoked scrollToBottom() exactly once when not editing; Esc closed the palette at document scope; /, N, and Esc inside the composer and a contenteditable="plaintext-only" target were ignored by the global dispatcher.

  • What was not tested: No additional gaps.

Audit

  • Audit A (existing helper): none found for editable-target dashboard shortcut guarding
  • Audit B (shared callers): 0 shared-helper callers; existing app-level dispatcher contract preserved
  • Audit C (broader rival): no open rival PR found for Control UI: add guarded dashboard keyboard shortcuts #81946

Tests

  • node scripts/run-vitest.mjs ui/src/ui/navigation.browser.test.ts
  • pnpm exec oxfmt --check ui/src/ui/app.ts ui/src/ui/navigation.browser.test.ts
  • pnpm tsgo:test:ui
  • pnpm test:changed --changed origin/main
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 14, 2026
@clawsweeper

clawsweeper Bot commented May 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
Adds a guarded document-level Control UI shortcut dispatcher for /, N, and Escape in ui/src/ui/app.ts, with browser coverage in ui/src/ui/navigation.browser.test.ts.

Reproducibility: not applicable. as a new Control UI shortcut feature rather than a broken existing contract. Source inspection of current main and the linked maintainer-scoped request show the pre-PR baseline and desired behavior.

Real behavior proof
Sufficient (live_output): Sufficient: the PR body includes copied live Chromium Control UI output showing /, N, and Escape behavior at document, composer, and plaintext-only contenteditable scopes.

Next step before merge
No automated repair is needed; maintainer review and landing are the remaining actions if required checks and the linked maintainer-scoped request are acceptable.

Security
Cleared: Cleared: the diff is limited to Control UI keyboard handling and a browser test, with no dependency, workflow, secret, permission, or package changes.

Review details

Best possible solution:

Land this PR after normal maintainer review and required checks, keeping the shortcut dispatcher scoped to the Control UI app shell with focused browser coverage for editable guards.

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

Not applicable as a new Control UI shortcut feature rather than a broken existing contract. Source inspection of current main and the linked maintainer-scoped request show the pre-PR baseline and desired behavior.

Is this the best way to solve the issue?

Yes. The PR uses the existing Control UI app shell and scroll/focus/tab paths, and the latest editable guard covers the previously missed contenteditable="plaintext-only" case.

What I checked:

  • PR implementation: The PR head adds a shared editable-target guard covering input, textarea, select, role textbox, CodeMirror, HTMLElement.isContentEditable, and contenteditable="plaintext-only", then routes /, N, and Escape through guarded document-level handling that reuses the existing composer focus, tab selection, mobile controls, session notice, focus-mode, and scroll paths. (ui/src/ui/app.ts:140, aa2c4762b381)
  • PR regression coverage: The new browser test covers non-chat N no-op, / switching from Overview to Chat and focusing the composer, N calling scrollToBottom, Escape closing the palette, Escape no-op in the composer, and //N/Escape no-op in a contenteditable="plaintext-only" target. (ui/src/ui/navigation.browser.test.ts:491, aa2c4762b381)
  • Current main shortcut baseline: Current main only has the existing document-level Cmd/Ctrl+K command-palette toggle and a separate mobile-controls Escape listener, so this PR is adding the new dashboard shortcut feature rather than duplicating existing main behavior. (ui/src/ui/app.ts:615, be166b9ae48d)
  • Current main new-message baseline: Current main renders the New messages affordance as a click-only button wired to onScrollToBottom, and app rendering shows that affordance is driven by chatNewMessagesBelow when no manual refresh is in flight. (ui/src/ui/views/chat.ts:1353, be166b9ae48d)
  • Linked maintainer-scoped request: The linked request asks for / composer focus, N new-message jump, and Escape dismissal while preserving composer-local behavior and no-op behavior in editable text targets, including contenteditable regions and component-owned editors.
  • Real proof and CI: The PR body now includes live Chromium Control UI output for /, N, and Escape at document scope, composer scope, and contenteditable="plaintext-only" scope; GitHub check runs for the head include successful Real behavior proof, core UI checks, test types, lint, build artifacts, and Socket PR alerts. (aa2c4762b381)

Likely related people:

  • BunsDev: Authored the linked maintainer-scoped shortcut request and has recent commits on ui/src/ui/app.ts covering Control UI chat/run status, auto-scroll, and text scaling near the affected surface. (role: feature scope owner and recent Control UI contributor; confidence: high; commits: 4935e24c7a7f, 256377c029f6, 52370c59980b; files: ui/src/ui/app.ts, ui/src/ui/app-render.ts, ui/src/ui/views/chat.ts)
  • shakkernerd: Recent GitHub path history for ui/src/ui/navigation.browser.test.ts shows focused navigation, mobile controls, and focus-shell test work adjacent to this PR's new browser coverage. (role: navigation test area contributor; confidence: medium; commits: b2f6e06e13f6, b8db4fc85932, b80f5c2ea55f; files: ui/src/ui/navigation.browser.test.ts)
  • Peter Steinberger: Local blame/log in this shallow checkout points the current app and navigation browser test lines to commit 7e9a863423; the commit appears broad, so this is a weaker routing signal than the feature and test-area history. (role: recent current-main committer; confidence: low; commits: 7e9a86342317; files: ui/src/ui/app.ts, ui/src/ui/navigation.browser.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against be166b9ae48d.

@stielemans

This comment was marked as low quality.

Sign in to view
@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 May 14, 2026
@hclsys

This comment was marked as low quality.

Sign in to view
@hclsys

This comment was marked as low quality.

Sign in to view
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 15, 2026
@hclsys

This comment was marked as low quality.

Sign in to view
@BunsDev BunsDev mentioned this pull request May 21, 2026
Open
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 proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Control UI: add guarded dashboard keyboard shortcuts

2 participants

@hclsys @stielemans