From afddc633a0e048fcec3b39169cc90e9122fc95f3 Mon Sep 17 00:00:00 2001 From: murashit Date: Wed, 27 May 2026 20:10:04 +0900 Subject: [PATCH] Modernize React UI DOM contracts --- src/features/chat/ui/composer.tsx | 11 +- src/features/chat/ui/message-stream.tsx | 83 +++++------- .../chat/ui/pending-request-message.tsx | 18 +-- styles.css | 123 +++++++++++------- tests/features/chat/ui/view-renderers.test.ts | 55 +++++--- 5 files changed, 158 insertions(+), 132 deletions(-) diff --git a/src/features/chat/ui/composer.tsx b/src/features/chat/ui/composer.tsx index e89df498..c99ac07f 100644 --- a/src/features/chat/ui/composer.tsx +++ b/src/features/chat/ui/composer.tsx @@ -110,12 +110,7 @@ function ComposerShell({ /> -
+
); } @@ -256,8 +251,8 @@ function ComposerSuggestions({ callbacks.onSuggestionInsert(suggestion); }} > - {suggestion.display} - {suggestion.detail ? {suggestion.detail} : null} + {suggestion.display} + {suggestion.detail ? {suggestion.detail} : null}
); })} diff --git a/src/features/chat/ui/message-stream.tsx b/src/features/chat/ui/message-stream.tsx index 15d6d35d..2328aea8 100644 --- a/src/features/chat/ui/message-stream.tsx +++ b/src/features/chat/ui/message-stream.tsx @@ -1,4 +1,4 @@ -import { useLayoutEffect, type ReactNode } from "react"; +import { useLayoutEffect, useRef, type ReactNode } from "react"; import { displayBlocksForItems } from "../display/blocks"; import { displayItemSignature, isMessageCopyActionVisible } from "../display/signature"; @@ -117,61 +117,40 @@ export function messageRenderBlocks(context: MessageStreamContext): MessageRende return blocks; } -export function syncMessageRenderBlocks(parent: HTMLElement, blocks: MessageRenderBlock[], signatures: Map): void { - const existing = new Map(); - parent.querySelectorAll(":scope > [data-codex-panel-block-key]").forEach((element) => { - const key = element.dataset["codexPanelBlockKey"]; - if (key) existing.set(key, element); - }); - - const seen = new Set(); - let nextPosition: ChildNode | null = parent.firstChild; - for (const block of blocks) { - const current = existing.get(block.key); - let element = current; - const currentWasNext = current === nextPosition; - if (!element || signatures.get(block.key) !== block.signature) { - element = block.render(); - element.dataset["codexPanelBlockKey"] = block.key; - element.dataset["codexPanelBlockSignature"] = shortSignature(block.signature); - signatures.set(block.key, block.signature); - if (current) { - current.replaceWith(element); - if (currentWasNext) nextPosition = element; - } - } - if (element !== nextPosition) { - parent.insertBefore(element, nextPosition); - } - nextPosition = element.nextSibling; - seen.add(block.key); - } - - for (const [key, element] of existing) { - if (!seen.has(key)) { - signatures.delete(key); - element.remove(); - } - } -} - export function renderMessageRenderBlocks(parent: HTMLElement, blocks: MessageRenderBlock[], signatures: Map): void { - renderReactRoot(parent, ); + renderReactRoot(parent, ); } -function MessageRenderBlockCommit({ - parent, - blocks, - signatures, -}: { - parent: HTMLElement; - blocks: MessageRenderBlock[]; - signatures: Map; -}): ReactNode { +function MessageRenderBlocks({ blocks, signatures }: { blocks: MessageRenderBlock[]; signatures: Map }): ReactNode { + return ( + <> + {blocks.map((block) => ( + + ))} + + ); +} + +function MessageRenderBlockHost({ block, signatures }: { block: MessageRenderBlock; signatures: Map }): ReactNode { + const ref = useRef(null); useLayoutEffect(() => { - syncMessageRenderBlocks(parent, blocks, signatures); - }, [parent, blocks, signatures]); - return null; + const host = ref.current; + if (!host) return; + if (signatures.get(block.key) !== block.signature) { + host.replaceChildren(block.render()); + signatures.set(block.key, block.signature); + } + host.dataset["codexPanelBlockSignature"] = shortSignature(block.signature); + }, [block, signatures]); + + useLayoutEffect(() => { + const key = block.key; + return () => { + signatures.delete(key); + }; + }, [block.key, signatures]); + + return
; } function createHistoryBarElement(loadingHistory: boolean, loadOlderTurns: () => void): HTMLElement { diff --git a/src/features/chat/ui/pending-request-message.tsx b/src/features/chat/ui/pending-request-message.tsx index 494c444d..63a623f8 100644 --- a/src/features/chat/ui/pending-request-message.tsx +++ b/src/features/chat/ui/pending-request-message.tsx @@ -84,12 +84,12 @@ function ApprovalCard({ }): ReactNode { return ( -
-
{approvalTitle(approval)}
-
{approvalSummary(approval)}
+
+
{approvalTitle(approval)}
+
{approvalSummary(approval)}
-
+
{approvalActionOptions(approval).map((option) => ( -
-
Codex needs input
-
+
+
Codex needs input
+
Answer {String(input.params.questions.length)} Plan mode question{input.params.questions.length === 1 ? "" : "s"} to continue.
-
+
{children}
; + return
{children}
; } function UserInputQuestions({ input, drafts }: { input: PendingUserInput; drafts: PendingRequestMessageDrafts }): ReactNode { diff --git a/styles.css b/styles.css index 3a9eb35e..f34b3104 100644 --- a/styles.css +++ b/styles.css @@ -120,6 +120,12 @@ flex: 0 0 auto; } +.codex-panel-threads__toolbar-actions { + display: flex; + align-items: center; + gap: var(--codex-panel-toolbar-button-gap); +} + .codex-panel-threads__list { display: flex; flex: 1 1 auto; @@ -238,36 +244,6 @@ opacity: 1; } -.codex-panel-threads__icon-button:hover, -.codex-panel-threads__icon-button:focus, -.codex-panel-threads__icon-button:focus-visible, -.codex-panel-threads__icon-button:active { - background: transparent; - box-shadow: none; - color: var(--icon-color-active); -} - -.codex-panel-threads__icon-button { - flex: 0 0 auto; - padding: var(--codex-panel-control-gap); -} - -.codex-panel-threads__toolbar-button { - --icon-size: var(--codex-panel-control-icon-size); - --icon-stroke: var(--icon-m-stroke-width, 1.75px); -} - -.codex-panel-threads__row-button { - --icon-size: var(--codex-panel-size-icon-s); - --icon-stroke: var(--icon-s-stroke-width, 1.75px); -} - -.codex-panel-threads__icon-button svg { - width: var(--icon-size); - height: var(--icon-size); - stroke-width: var(--icon-stroke); -} - .codex-panel-threads__rename-form { display: flex; align-items: center; @@ -344,20 +320,43 @@ color: var(--codex-panel-text-normal); } -.codex-panel-ui__toolbar-control { +.codex-panel-ui__toolbar-control, +.codex-panel-ui__icon-button, +.codex-panel-threads__icon-button { box-sizing: border-box; + display: inline-flex; + flex: 0 0 auto; + align-items: center; + justify-content: center; + appearance: none; + border: 0; + background: transparent; + box-shadow: none; + color: var(--icon-color); + font: inherit; + line-height: var(--line-height-tight); + cursor: pointer; + user-select: none; +} + +.codex-panel-ui__toolbar-control:disabled, +.codex-panel-ui__icon-button:disabled, +.codex-panel-threads__icon-button:disabled { + cursor: default; + opacity: 0.45; +} + +.codex-panel-ui__toolbar-control { --icon-size: var(--codex-panel-control-icon-size); --icon-stroke: var(--icon-m-stroke-width, 1.75px); min-width: var(--codex-panel-icon-button-inline-size); width: auto; height: auto; padding: var(--codex-panel-control-gap) var(--codex-panel-item-gap); - border: 0; border-radius: var(--codex-panel-control-radius); } .codex-panel-ui__icon-button { - box-sizing: border-box; --icon-size: var(--codex-panel-control-icon-size); --icon-stroke: var(--icon-m-stroke-width, 1.75px); min-width: var(--codex-panel-composer-control-size); @@ -367,17 +366,39 @@ border-radius: var(--codex-panel-control-radius); } +.codex-panel-threads__icon-button { + --icon-size: var(--codex-panel-control-icon-size); + --icon-stroke: var(--icon-m-stroke-width, 1.75px); + padding: var(--codex-panel-control-gap); +} + +.codex-panel-threads__toolbar-button { + --icon-size: var(--codex-panel-control-icon-size); + --icon-stroke: var(--icon-m-stroke-width, 1.75px); +} + +.codex-panel-threads__row-button { + --icon-size: var(--codex-panel-size-icon-s); + --icon-stroke: var(--icon-s-stroke-width, 1.75px); +} + +.codex-panel-threads__icon-button:hover, +.codex-panel-threads__icon-button:focus, +.codex-panel-threads__icon-button:focus-visible, +.codex-panel-threads__icon-button:active { + background: transparent; + box-shadow: none; + color: var(--icon-color-active); +} + .codex-panel-ui__toolbar-control svg, -.codex-panel-ui__icon-button svg { +.codex-panel-ui__icon-button svg, +.codex-panel-threads__icon-button svg { width: var(--icon-size); height: var(--icon-size); stroke-width: var(--icon-stroke); } -.codex-panel-ui__icon-button:disabled { - opacity: 0.45; -} - .codex-panel-ui__text-input { width: 100%; overflow: hidden; @@ -457,7 +478,7 @@ background: var(--background-modifier-active-hover); } -.codex-panel .clickable-icon.codex-panel__runtime-model { +.codex-panel__runtime-model { display: inline-flex; align-items: center; flex: 0 1 auto; @@ -478,16 +499,16 @@ user-select: none; } -.codex-panel .clickable-icon.codex-panel__runtime-model:hover, -.codex-panel .clickable-icon.codex-panel__runtime-model.is-active { +.codex-panel__runtime-model:hover, +.codex-panel__runtime-model.is-active { color: var(--text-normal); } -.codex-panel .clickable-icon.codex-panel__runtime-model.is-active { +.codex-panel__runtime-model.is-active { background: var(--background-modifier-hover); } -.codex-panel .clickable-icon.codex-panel__runtime-model:hover { +.codex-panel__runtime-model:hover { background: var(--background-modifier-hover); } @@ -1298,6 +1319,10 @@ margin-bottom: var(--codex-panel-section-gap); } +.codex-panel__message-block { + display: flow-root; +} + .codex-panel__message { margin-bottom: var(--codex-panel-edge-padding-x); padding-left: var(--codex-panel-message-indent); @@ -1637,7 +1662,8 @@ font-family: var(--font-interface); } -.codex-panel__pending-request-card.setting-item { +.codex-panel__pending-request-card { + display: flex; flex-direction: column; align-items: stretch; gap: var(--codex-panel-control-gap); @@ -1647,12 +1673,12 @@ background: transparent; } -.codex-panel__pending-request-card.setting-item + .codex-panel__pending-request-card.setting-item { +.codex-panel__pending-request-card + .codex-panel__pending-request-card { padding-top: var(--codex-panel-item-gap); border-top: 1px solid var(--codex-panel-border-muted-color); } -.codex-panel__pending-request-info.setting-item-info { +.codex-panel__pending-request-info { min-width: 0; margin-right: 0; } @@ -1677,7 +1703,8 @@ font-size: var(--font-ui-smaller); } -.codex-panel__pending-request-actions.setting-item-control { +.codex-panel__pending-request-actions { + display: flex; justify-content: flex-start; flex-wrap: wrap; gap: var(--codex-panel-control-gap); @@ -2163,7 +2190,7 @@ color: var(--icon-color-active); } -.codex-panel__composer-suggestions.suggestion-container { +.codex-panel__composer-suggestions { position: absolute; right: var(--codex-panel-edge-padding-x); bottom: calc(100% - var(--codex-panel-control-gap)); diff --git a/tests/features/chat/ui/view-renderers.test.ts b/tests/features/chat/ui/view-renderers.test.ts index b9d33e90..fa1a36e0 100644 --- a/tests/features/chat/ui/view-renderers.test.ts +++ b/tests/features/chat/ui/view-renderers.test.ts @@ -15,12 +15,13 @@ import { renderPendingRequestMessage } from "../../../../src/features/chat/ui/pe import { renderToolbar, toolbarSignature, type ToolbarViewModel } from "../../../../src/features/chat/ui/toolbar"; import { displayItemSignature } from "../../../../src/features/chat/display/signature"; import { implementPlanCandidateFromState } from "../../../../src/features/chat/chat-message-renderer"; -import { messageRenderBlocks as rawMessageRenderBlocks, syncMessageRenderBlocks } from "../../../../src/features/chat/ui/message-stream"; +import { messageRenderBlocks as rawMessageRenderBlocks, renderMessageRenderBlocks } from "../../../../src/features/chat/ui/message-stream"; import { displayDiffLines, persistedChatTurnDiffViewState, renderChatTurnDiffView } from "../../../../src/features/chat/ui/turn-diff"; import { composerSuggestionScrollFixture, installObsidianDomShims, topLevelDetailsSummaries } from "./dom-test-helpers"; import { renderThreadsView } from "../../../../src/features/threads-view/renderer"; import { liveStateForSnapshots, threadRows, type ThreadsRowModel } from "../../../../src/features/threads-view/state"; import type { Thread } from "../../../../src/generated/app-server/v2/Thread"; +import { unmountReactRoot } from "../../../../src/shared/ui/react-root"; installObsidianDomShims(); @@ -153,31 +154,47 @@ describe("message stream block identity and message actions", () => { ); }); - it("reuses message block nodes while signatures are stable", () => { + it("reuses React message block hosts while signatures are stable", () => { const parent = document.createElement("div"); const signatures = new Map(); const first = document.createElement("section"); first.textContent = "first"; + const firstRender = vi.fn(() => first); - syncMessageRenderBlocks(parent, [{ key: "one", signature: "same", render: () => first }], signatures); - syncMessageRenderBlocks(parent, [{ key: "one", signature: "same", render: () => document.createElement("aside") }], signatures); + renderMessageRenderBlocks(parent, [{ key: "one", signature: "same", render: firstRender }], signatures); - expect(parent.firstElementChild).toBe(first); + const host = expectPresent(parent.querySelector('[data-codex-panel-block-key="one"]')); + expect(host.classList.contains("codex-panel__message-block")).toBe(true); + expect(host.firstElementChild).toBe(first); + expect(firstRender).toHaveBeenCalledOnce(); + + const skippedRender = vi.fn(() => document.createElement("aside")); + renderMessageRenderBlocks(parent, [{ key: "one", signature: "same", render: skippedRender }], signatures); + + expect(parent.querySelector('[data-codex-panel-block-key="one"]')).toBe(host); + expect(host.firstElementChild).toBe(first); + expect(skippedRender).not.toHaveBeenCalled(); const replacement = document.createElement("article"); - syncMessageRenderBlocks(parent, [{ key: "one", signature: "changed", render: () => replacement }], signatures); + renderMessageRenderBlocks(parent, [{ key: "one", signature: "changed", render: () => replacement }], signatures); - expect(parent.firstElementChild).toBe(replacement); + expect(parent.querySelector('[data-codex-panel-block-key="one"]')).toBe(host); + expect(host.firstElementChild).toBe(replacement); expect(signatures.get("one")).toBe("changed"); + + renderMessageRenderBlocks(parent, [], signatures); + expect(parent.querySelector('[data-codex-panel-block-key="one"]')).toBeNull(); + expect(signatures.has("one")).toBe(false); + unmountReactRoot(parent); }); - it("leaves stable ordered message block nodes in place during repeated syncs", () => { + it("leaves stable ordered React message block hosts in place during repeated renders", () => { const parent = document.createElement("div"); const signatures = new Map(); const first = document.createElement("section"); const second = document.createElement("article"); - syncMessageRenderBlocks( + renderMessageRenderBlocks( parent, [ { key: "one", signature: "same-one", render: () => first }, @@ -185,19 +202,26 @@ describe("message stream block identity and message actions", () => { ], signatures, ); + const firstHost = expectPresent(parent.querySelector('[data-codex-panel-block-key="one"]')); + const secondHost = expectPresent(parent.querySelector('[data-codex-panel-block-key="two"]')); const insertBefore = vi.spyOn(parent, "insertBefore"); - syncMessageRenderBlocks( + const skippedRender = vi.fn(() => document.createElement("aside")); + renderMessageRenderBlocks( parent, [ - { key: "one", signature: "same-one", render: () => document.createElement("aside") }, - { key: "two", signature: "same-two", render: () => document.createElement("aside") }, + { key: "one", signature: "same-one", render: skippedRender }, + { key: "two", signature: "same-two", render: skippedRender }, ], signatures, ); expect(insertBefore).not.toHaveBeenCalled(); - expect([...parent.children]).toEqual([first, second]); + expect(skippedRender).not.toHaveBeenCalled(); + expect([...parent.children]).toEqual([firstHost, secondHost]); + expect(firstHost.firstElementChild).toBe(first); + expect(secondHost.firstElementChild).toBe(second); + unmountReactRoot(parent); }); it("inserts completed-turn activity groups without replacing stable conversation nodes", () => { @@ -215,7 +239,7 @@ describe("message stream block identity and message actions", () => { renderTextWithWikiLinks: (element: HTMLElement, text: string) => element.createDiv({ text }), }; - syncMessageRenderBlocks( + renderMessageRenderBlocks( parent, messageRenderBlocks({ ...baseContext, @@ -229,7 +253,7 @@ describe("message stream block identity and message actions", () => { const userNode = parent.querySelector('[data-codex-panel-block-key="item:u1"]'); const assistantNode = parent.querySelector('[data-codex-panel-block-key="item:a1"]'); - syncMessageRenderBlocks( + renderMessageRenderBlocks( parent, messageRenderBlocks({ ...baseContext, @@ -260,6 +284,7 @@ describe("message stream block identity and message actions", () => { expect(parent.querySelector('[data-codex-panel-block-key="activity:turn-t1-activity"] summary')?.textContent).toBe( "Work details: hook", ); + unmountReactRoot(parent); }); it("does not invalidate generic tool blocks when only the workspace root changes", () => {