From 8660ece6c15cdfa4fb27903e7abff281f067a66a Mon Sep 17 00:00:00 2001 From: rookiestar28 Date: Sun, 26 Apr 2026 17:52:34 +0800 Subject: [PATCH] feat(frontend): add shared DOM wiring helpers --- docs/frontend-tab-wiring.md | 30 +++++++++ tests/test_f59_slack_interactions.py | 14 ++-- web/openclaw_api.js | 34 +++++++++- web/openclaw_tabs.js | 40 ++++++++---- web/openclaw_utils.js | 75 ++++++++++++++++++++++ web/tests/unit/openclaw_tabs.test.js | 92 +++++++++++++++++++++++++++ web/tests/unit/openclaw_utils.test.js | 44 +++++++++++++ 7 files changed, 309 insertions(+), 20 deletions(-) create mode 100644 docs/frontend-tab-wiring.md create mode 100644 web/tests/unit/openclaw_tabs.test.js diff --git a/docs/frontend-tab-wiring.md b/docs/frontend-tab-wiring.md new file mode 100644 index 0000000..f094359 --- /dev/null +++ b/docs/frontend-tab-wiring.md @@ -0,0 +1,30 @@ +# Frontend Tab Wiring + +OpenClaw's frontend uses modular vanilla ES modules loaded by the ComfyUI extension host. Keep tab work inside this architecture unless a future migration decision explicitly changes the runtime model. + +## Tab Registration + +- Register tabs through `tabManager.registerTab({ id, title, icon, render })`. +- Keep `id` stable; it is used for pane ids and active-tab storage. +- Treat `render(pane)` as the only place that mutates a tab pane. +- Return a promise from `render` only when the tab genuinely performs async work; async failures are routed through the tab error boundary. + +## DOM Helpers + +- Prefer shared helpers from `web/openclaw_utils.js` for new shell/tab wiring: + - `createDomElement(...)` for text-safe element construction. + - `appendChildren(...)` for optional child nodes. + - `queryRequired(...)` when a selector is mandatory for the tab to function. +- Use `textContent` semantics for user-visible text. Do not add raw HTML helper paths for convenience. +- Keep legacy class aliasing centralized through existing normalization and alias helpers. + +## API Contracts + +- Use `OpenClawAPI.fetch(...)` normalized results instead of direct `fetch` from tabs. +- Check `result.ok` before reading `result.data`. +- Preserve admin-token handling inside `OpenClawAPI` and shared session helpers. + +## Verification + +- Add Vitest coverage for new shared helpers or tab wiring behavior. +- Use Playwright harness specs for user-visible tab behavior such as active panes, rendered content, and action outcomes. diff --git a/tests/test_f59_slack_interactions.py b/tests/test_f59_slack_interactions.py index c5f44ce..92c3ef1 100644 --- a/tests/test_f59_slack_interactions.py +++ b/tests/test_f59_slack_interactions.py @@ -21,10 +21,7 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from connector.config import ConnectorConfig, load_config from connector.contract import CommandResponse -from connector.platforms.slack_webhook import ( - SLACK_SIGNING_VERSION, - SlackWebhookServer, -) +from connector.platforms.slack_webhook import SLACK_SIGNING_VERSION, SlackWebhookServer SIGNING_SECRET = "test_signing_secret_f59" @@ -155,7 +152,10 @@ class TestF59SlackInteractions(unittest.IsolatedAsyncioTestCase): self.assertEqual(routed.text, "/approvals") self.assertTrue(routed.metadata["interactive_callback"]) self.assertEqual(routed.metadata["interaction_type"], "block_actions") - self.assertEqual(routed.metadata["response_url"], "https://hooks.slack.com/actions/T_F59/mock") + self.assertEqual( + routed.metadata["response_url"], + "https://hooks.slack.com/actions/T_F59/mock", + ) async def test_invalid_signature_rejected_without_routing(self): server = _make_server() @@ -171,7 +171,9 @@ class TestF59SlackInteractions(unittest.IsolatedAsyncioTestCase): async def test_duplicate_block_action_is_acknowledged_once_without_reroute(self): server = _make_server() - payload = _block_action_payload(value="/status", action_id="dup", trigger_id="t-dup") + payload = _block_action_payload( + value="/status", action_id="dup", trigger_id="t-dup" + ) req1 = _build_interaction_request(payload) req2 = _build_interaction_request(payload) diff --git a/web/openclaw_api.js b/web/openclaw_api.js index 0f3a132..86c2cbc 100644 --- a/web/openclaw_api.js +++ b/web/openclaw_api.js @@ -14,6 +14,35 @@ import { withPreconnectHint, } from "./openclaw_fetch_wrappers.js"; +/** + * @typedef {Object} OpenClawFetchOptions + * @property {number=} timeout Request timeout in milliseconds. + * @property {AbortSignal=} signal Optional caller-owned cancellation signal. + * @property {string=} method HTTP method. + * @property {HeadersInit=} headers Request headers. + * @property {BodyInit|null=} body Request body. + */ + +/** + * @typedef {Object} OpenClawFetchSuccess + * @property {true} ok + * @property {number} status + * @property {*} data Parsed JSON value or response text. + */ + +/** + * @typedef {Object} OpenClawFetchFailure + * @property {false} ok + * @property {number} status HTTP status, or 0 for network/timeout/cancelled failures. + * @property {string} error Stable error code/message. + * @property {*=} data Parsed error payload or response text. + * @property {string=} detail Low-level error detail for diagnostics. + */ + +/** + * @typedef {OpenClawFetchSuccess|OpenClawFetchFailure} OpenClawFetchResult + */ + export class OpenClawAPI { constructor() { this._capabilitiesCache = null; @@ -77,9 +106,8 @@ export class OpenClawAPI { /** * Generic fetch wrapper with timeout and error normalization. * @param {string} url - The URL to fetch - * @param {object} options - Fetch options - * @param {number} options.timeout - Timeout in ms (default: 10000) - * @param {AbortSignal} options.signal - Optional AbortSignal from caller (R38-Lite) + * @param {OpenClawFetchOptions} options - Fetch options + * @returns {Promise} */ async fetch(url, options = {}) { const { timeout = 10000, signal: externalSignal, ...fetchOptions } = options; diff --git a/web/openclaw_tabs.js b/web/openclaw_tabs.js index ca8282f..7367126 100644 --- a/web/openclaw_tabs.js +++ b/web/openclaw_tabs.js @@ -6,11 +6,23 @@ import { ErrorBoundary } from "./ErrorBoundary.js"; import { STORAGE_KEYS, getMirroredStorageValue, setMirroredStorageValue } from "./openclaw_compat.js"; import { applyLegacyClassAliases, + appendChildren, + createDomElement, normalizeLegacyClassNames, } from "./openclaw_utils.js"; +/** + * @typedef {Object} OpenClawTabDefinition + * @property {string} id Stable tab id used for storage and pane ids. + * @property {string} title Visible tab label and error-boundary name. + * @property {string=} icon Optional icon class for the tab button. + * @property {(pane: HTMLElement) => (void|Promise)} render Render function for the tab pane. + * @property {boolean=} loaded Internal lazy-render state. + */ + export class TabManager { constructor() { + /** @type {OpenClawTabDefinition[]} */ this.tabs = [ // { id, title, renderFn, loaded } ]; @@ -31,6 +43,10 @@ export class TabManager { this._restoreActiveTab(); } + /** + * Register or replace a tab definition. + * @param {OpenClawTabDefinition} tabDef + */ registerTab(tabDef) { // Idempotent: replace existing tab definition by id const idx = this.tabs.findIndex(t => t.id === tabDef.id); @@ -53,16 +69,17 @@ export class TabManager { this.tabsEl.innerHTML = ""; this.tabs.forEach(tab => { - const btn = document.createElement("div"); - btn.className = "openclaw-tab"; + const btn = createDomElement("div", { className: "openclaw-tab" }); if (tab.icon) { - const icon = document.createElement("i"); - icon.className = `openclaw-tab-icon ${tab.icon}`; - const label = document.createElement("span"); - label.className = "openclaw-tab-label"; - label.textContent = tab.title; - btn.appendChild(icon); - btn.appendChild(label); + appendChildren(btn, [ + createDomElement("i", { + className: `openclaw-tab-icon ${tab.icon}`, + }), + createDomElement("span", { + className: "openclaw-tab-label", + text: tab.title, + }), + ]); } else { btn.textContent = tab.title; } @@ -73,9 +90,10 @@ export class TabManager { // Create container for tab content if not exists if (!this.contentEl.querySelector(`#openclaw-tab-${tab.id}`)) { - const pane = document.createElement("div"); + const pane = createDomElement("div", { + className: "openclaw-tab-pane", + }); pane.id = `openclaw-tab-${tab.id}`; - pane.className = "openclaw-tab-pane"; this.contentEl.appendChild(pane); } }); diff --git a/web/openclaw_utils.js b/web/openclaw_utils.js index 2ad7e36..d9c0c41 100644 --- a/web/openclaw_utils.js +++ b/web/openclaw_utils.js @@ -19,6 +19,81 @@ export function makeEl(tag, className = "", text = "") { return el; } +/** + * Build a DOM element from a small declarative option bag. + * Text is assigned with textContent; this helper intentionally has no raw HTML path. + * + * @param {string} tag + * @param {{ + * className?: string|string[], + * text?: string|number|boolean|null, + * attrs?: Record, + * dataset?: Record, + * children?: Array|Node|null|undefined, + * }} options + * @returns {HTMLElement} + */ +export function createDomElement(tag, options = {}) { + const el = document.createElement(tag); + const className = options.className; + if (Array.isArray(className)) { + el.className = className.filter(Boolean).join(" "); + } else if (className) { + el.className = String(className); + } + + if (options.text !== undefined && options.text !== null) { + el.textContent = String(options.text); + } + + Object.entries(options.attrs || {}).forEach(([name, value]) => { + if (value === undefined || value === null) return; + el.setAttribute(name, String(value)); + }); + + Object.entries(options.dataset || {}).forEach(([name, value]) => { + if (value === undefined || value === null) return; + el.dataset[name] = String(value); + }); + + appendChildren(el, options.children || []); + return el; +} + +/** + * Append only real DOM nodes, allowing callers to keep optional branches concise. + * @param {Node} parent + * @param {Array|Node|null|undefined} children + * @returns {Node} + */ +export function appendChildren(parent, children = []) { + const items = Array.isArray(children) ? children : [children]; + items.flat().forEach((child) => { + if (child instanceof Node) { + parent.appendChild(child); + } + }); + return parent; +} + +/** + * Query a required descendant and fail with stable owner context when missing. + * @template {Element} T + * @param {ParentNode|null|undefined} root + * @param {string} selector + * @param {string} owner + * @returns {T} + */ +export function queryRequired(root, selector, owner = "OpenClaw UI") { + const found = root && typeof root.querySelector === "function" + ? root.querySelector(selector) + : null; + if (!found) { + throw new Error(`${owner} missing required selector: ${selector}`); + } + return found; +} + /** * F63: prefer canonical `openclaw-*` classes when both legacy and canonical * variants are present on the same node. diff --git a/web/tests/unit/openclaw_tabs.test.js b/web/tests/unit/openclaw_tabs.test.js new file mode 100644 index 0000000..06efeed --- /dev/null +++ b/web/tests/unit/openclaw_tabs.test.js @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TabManager } from "../../openclaw_tabs.js"; + +function makeMount() { + const tabsEl = document.createElement("div"); + const contentEl = document.createElement("div"); + document.body.appendChild(tabsEl); + document.body.appendChild(contentEl); + return { tabsEl, contentEl }; +} + +describe("TabManager", () => { + beforeEach(() => { + document.body.innerHTML = ""; + localStorage.clear(); + }); + + it("renders canonical tab buttons and creates matching panes", () => { + const manager = new TabManager(); + const { tabsEl, contentEl } = makeMount(); + + manager.registerTab({ + id: "settings", + title: "Settings", + icon: "openclaw-icon-settings", + render(pane) { + pane.textContent = "Settings loaded"; + }, + }); + manager.init(tabsEl, contentEl); + + const button = tabsEl.querySelector(".openclaw-tab"); + expect(button.querySelector(".openclaw-tab-icon").className).toContain( + "openclaw-icon-settings" + ); + expect(button.querySelector(".openclaw-tab-label").textContent).toBe("Settings"); + + const pane = contentEl.querySelector("#openclaw-tab-settings"); + expect(pane.className).toContain("openclaw-tab-pane"); + expect(pane.classList.contains("active")).toBe(true); + expect(pane.textContent).toBe("Settings loaded"); + }); + + it("switches active state without duplicating panes", () => { + const manager = new TabManager(); + const { tabsEl, contentEl } = makeMount(); + + manager.registerTab({ + id: "settings", + title: "Settings", + render(pane) { + pane.textContent = "Settings loaded"; + }, + }); + manager.registerTab({ + id: "jobs", + title: "Jobs", + render(pane) { + pane.textContent = "Jobs loaded"; + }, + }); + manager.init(tabsEl, contentEl); + manager.activateTab("jobs"); + + expect(contentEl.querySelectorAll(".openclaw-tab-pane")).toHaveLength(2); + expect(contentEl.querySelector("#openclaw-tab-settings").classList.contains("active")).toBe(false); + expect(contentEl.querySelector("#openclaw-tab-jobs").classList.contains("active")).toBe(true); + expect(contentEl.querySelector("#openclaw-tab-jobs").textContent).toBe("Jobs loaded"); + }); + + it("routes async render failures through the tab error boundary", async () => { + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const manager = new TabManager(); + const { tabsEl, contentEl } = makeMount(); + + manager.registerTab({ + id: "broken", + title: "Broken", + render() { + return Promise.reject(new Error("async tab failed")); + }, + }); + manager.init(tabsEl, contentEl); + await Promise.resolve(); + + const pane = contentEl.querySelector("#openclaw-tab-broken"); + expect(pane.querySelector(".openclaw-error-boundary code").textContent).toContain( + "async tab failed" + ); + consoleSpy.mockRestore(); + }); +}); diff --git a/web/tests/unit/openclaw_utils.test.js b/web/tests/unit/openclaw_utils.test.js index f206904..808a7d3 100644 --- a/web/tests/unit/openclaw_utils.test.js +++ b/web/tests/unit/openclaw_utils.test.js @@ -1,12 +1,15 @@ import { describe, expect, it } from "vitest"; import { + appendChildren, applyLegacyClassAliases, buildLegacyAliasClassTokens, + createDomElement, makeEl, normalizeLegacyClassTokens, normalizeLegacyClassNames, parseJsonSafe, parseJsonOrThrow, + queryRequired, isAbortError, } from "../../openclaw_utils.js"; @@ -18,6 +21,47 @@ describe("openclaw_utils", () => { expect(el.textContent).toBe("Hello"); }); + it("creates declarative DOM elements without treating text as HTML", () => { + const child = createDomElement("span", { + className: "openclaw-label", + text: "Run", + dataset: { role: "primary" }, + attrs: { title: "Run command", "aria-live": "polite" }, + }); + const root = createDomElement("div", { + className: "openclaw-row", + children: [child], + }); + + expect(root.className).toBe("openclaw-row"); + expect(root.firstElementChild).toBe(child); + expect(child.textContent).toBe("Run"); + expect(child.innerHTML).toBe("<strong>Run</strong>"); + expect(child.dataset.role).toBe("primary"); + expect(child.getAttribute("aria-live")).toBe("polite"); + }); + + it("appends only defined child nodes", () => { + const root = document.createElement("div"); + const first = document.createElement("span"); + const second = document.createElement("button"); + + appendChildren(root, [first, null, undefined, second]); + + expect(Array.from(root.children)).toEqual([first, second]); + }); + + it("queries required elements with stable owner context", () => { + const root = createDomElement("section", { + children: [createDomElement("button", { attrs: { id: "run" }, text: "Run" })], + }); + + expect(queryRequired(root, "#run", "Planner tab").textContent).toBe("Run"); + expect(() => queryRequired(root, "#missing", "Planner tab")).toThrow( + "Planner tab missing required selector: #missing" + ); + }); + it("normalizes duplicate legacy class tokens", () => { expect( normalizeLegacyClassTokens("openclaw-btn moltbot-btn openclaw-btn-primary moltbot-btn-primary openclaw-btn")