Skip to content

feat(web-shell): ui优化#4759

Merged
ytahdn merged 1 commit into
QwenLM:daemon_mode_b_mainfrom
chiga0:feat/web-shell-ui-polish
Jun 4, 2026
Merged

feat(web-shell): ui优化#4759
ytahdn merged 1 commit into
QwenLM:daemon_mode_b_mainfrom
chiga0:feat/web-shell-ui-polish

Conversation

@ytahdn

@ytahdn ytahdn commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • 新增 web-shell 组件定制能力:支持隐藏指定斜杠命令、定制工具卡片 header 后半段内容,并允许对 assistant 正文 Markdown 做定制处理。
  • 将 /approval-mode 从独立弹窗调整为消息流内联面板,并移除 web-shell 自定义的 /mode 命令,避免与 CLI 行为不一致。
  • 调整 /clear 行为为创建新 session;紧凑模式切换不再输出提示,并持久化到 localStorage,刷新后保持设置。
  • 优化 resume/delete/release 会话面板交互:会话标题与元信息按两行展示,长标题不再挤压右侧信息;禁用 hover 选中以避免鼠标与键盘导航互相抢焦点。
  • 补齐相关国际化文案与导出的 TypeScript 类型。

Test Plan

  • 已通过 web-shell 构建:npm run build(packages/web-shell)。
  • 提交前 pre-commit 已运行 prettier 与 eslint。

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR introduces UI polish for the embedded web-shell, including a new customization API for tool headers and Markdown rendering, conversion of approval-mode from dialog to inline panel, and various UX improvements to session management dialogs. The changes are well-structured with good TypeScript typing and consistent patterns.

🔍 General Feedback

  • Good modularization: New customization.tsx provides a clean extension point for host applications to customize tool headers and Markdown rendering without modifying core components.
  • Consistent keyboard navigation: All dialogs and the new approval-mode panel follow the same keyboard interaction patterns (arrow keys, Enter, Escape, j/k bindings).
  • Proper cleanup patterns: useEffect cleanup functions are correctly implemented for event emitters and async operations.
  • Good use of React patterns: Appropriate use of useMemo, useCallback, refs for mutable state, and context for dependency injection.
  • Type safety: New types are well-defined and exported through the public API (index.ts).

🎯 Specific Feedback

🟡 High

  1. Security: Markdown customization could introduce XSS if not carefully used

    • File: packages/web-shell/client/components/messages/Markdown.tsx:456-465 - The transformMarkdown function allows external code to modify Markdown content before rendering. While the existing sanitizeSvg and isSafeHref functions are robust, custom components/rehype plugins injected via markdown.components or markdown.rehypePlugins bypass the built-in sanitization. Consider adding documentation warnings or a sanitization hook that runs after transformation.
  2. State management: Approval mode panel uses both state and event-based visibility

    • File: packages/web-shell/client/App.tsx:615,657-663 - The approval mode visibility is tracked via approvalModeInlineOpen state AND approvalModePanelActive from panel events. This dual mechanism could lead to edge cases where the panel state diverges. The pattern is similar to other panels (MCP, agents, memory), so it's consistent, but ensure the APPROVAL_MODE_ACTIVE_EVENT dispatch timing matches the component mount/unmount cycle.

🟢 Medium

  1. Accessibility: Missing ARIA attributes on custom select panel

    • File: packages/web-shell/client/components/messages/ApprovalModeMessage.tsx:113-138 - The approval mode list uses custom keyboard navigation but lacks role="listbox", role="option", aria-selected, and aria-activedescendant attributes. This impacts screen reader users. Consider adding these ARIA attributes to match the semantic behavior.
  2. CSS: Inconsistent margin specification

    • File: packages/web-shell/client/components/messages/ApprovalModeMessage.module.css:1-14 - The .panel rule has margin: 0px 0 12px 14px followed by margin-left: 14px on line 14, which is redundant. Also, mixing 0px and 0 is inconsistent. Simplify to margin: 0 0 12px 14px.
  3. Type safety: ToolHeaderExtraRenderer could be more restrictive

    • File: packages/web-shell/client/customization.tsx:30-37 - The ToolHeaderExtraRenderInfo exposes the full ACPToolCall type to custom renderers. Consider exposing only the necessary fields (toolName, status, description) rather than the entire ACP type, which may change and break custom implementations.
  4. Error handling: Silent localStorage failures

    • File: packages/web-shell/client/App.tsx:120-129 - The saveCompactMode and loadCompactMode functions silently catch errors. While the comment mentions private browsing, consider at least logging to console in development mode (if (import.meta.env.DEV) console.warn(...)) to help developers debug unexpected behavior.
  5. Performance: Mermaid theme re-initialization

    • File: packages/web-shell/client/components/messages/Markdown.tsx:176-181 - The lastMermaidTheme module-level variable prevents redundant mermaid.initialize() calls, which is good. However, this is a module-level mutable variable that could cause issues in SSR or testing environments with module resets. Consider using a React ref in a context provider instead.

🔵 Low

  1. Documentation: Missing JSDoc for new public API types

    • File: packages/web-shell/client/customization.tsx - The exported types (WebShellMarkdownCustomization, ToolHeaderExtraRenderer, etc.) lack JSDoc comments explaining their purpose and usage. Add documentation since these are part of the public API exported from index.ts.
  2. Naming: Inconsistent event naming convention

    • File: packages/web-shell/client/components/messages/ApprovalModeMessage.tsx:7 - APPROVAL_MODE_ACTIVE_EVENT uses the pattern web-shell:approval-mode-panel-active, which matches other panel events. However, the variable name uses APPROVAL_MODE while the event string says approval-mode-panel. Consider aligning: either APPROVAL_MODE_PANEL_ACTIVE_EVENT or change the event string.
  3. Code reuse: Duplicate mode translation logic

    • File: packages/web-shell/client/components/messages/ApprovalModeMessage.tsx:33-37 - The mode mapping (DAEMON_APPROVAL_MODES.map) creates {id, name, description} objects. This same pattern may be used elsewhere (e.g., the old ApprovalModeDialog). Extract to a shared utility function if not already present.
  4. i18n: Missing translation for new mode descriptions

    • File: packages/web-shell/client/i18n.tsx:555-566 - New mode translations are added (mode.name.*, mode.desc.*), but ensure these are also added to the Chinese translations section (lines 1216+) if not already done. The diff shows they were added, but verify completeness.
  5. Testing: No unit tests for new customization hooks

    • While this is a UI polish PR, the new useWebShellCustomization hook and customization context would benefit from basic unit tests to ensure the context provides defaults correctly and handles undefined props gracefully.

✅ Highlights

  • Excellent customization API design: The WebShellCustomizationProvider pattern allows host applications to inject custom behavior without fork modifications. This is a mature, well-thought-out extension point.
  • Clean inline panel migration: Moving /approval-mode from dialog to inline panel improves UX consistency with other panel-based commands (MCP, agents, memory). The keyboard navigation is well-implemented.
  • Thoughtful CSS architecture: The approval mode panel CSS uses CSS variables (--accent-color, --text-primary, etc.) for theming, maintains consistent spacing with other dialogs, and includes responsive max-height calculations.
  • Good type exports: All new public types are properly exported from both index.ts and index.tsx, ensuring consumers have full type information.
  • Proper React memoization: The customization object in App.tsx uses useMemo with correct dependencies, preventing unnecessary re-renders of child components.

@ytahdn ytahdn requested review from chiga0, wenshao and yiliang114 June 4, 2026 06:28
@ytahdn ytahdn changed the title feat(web-shell): 优化嵌入式终端交互 feat(web-shell): ui优化 Jun 4, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] 旧的 ApprovalModeDialog.tsx 已成为死代码——PR 将其替换为内联的 ApprovalModeMessage,但旧文件未删除,整个 codebase 中无任何文件 import 它。旧的 i18n key(mode.plan.descmode.default.desc 等 10 个 EN+ZH 条目)也不再被使用,与新增的 mode.desc.* key 容易混淆。建议一并清理。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/App.tsx
@wenshao

wenshao commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Local Verification Report — PR #4759

Tested on: macOS Darwin 25.4.0 (Apple Silicon)
Branch: feat/web-shell-ui-polish @ 03f44d4
Base: daemon_mode_b_main
Tester: wenshao


Test Results Summary

Test Suite Result Details
packages/web-shell vitest (unit tests) PASS 92/93 tests pass; 1 pre-existing failure (see below)
vite build --config vite.lib.config.ts (lib build) PASS 105 modules, 514.71 kB output
vite build (app build) ⚠️ PRE-EXISTING FAIL Missing @qwen-code/webui/daemon-react-sdk — fails on base branch too
eslint packages/web-shell/client PASS 0 errors
prettier --check (PR-modified files) PASS All PR files formatted correctly
tsc -p tsconfig.lib.json ⚠️ PRE-EXISTING TS errors in files not modified by this PR (see below)

Pre-Existing Issues (NOT introduced by this PR)

All failures below were verified to exist on the base branch (daemon_mode_b_main) without this PR:

  1. App build fail@qwen-code/webui/daemon-react-sdk module not found. The lib build (which externalizes this dep) passes fine.
  2. TypeScript errorsTS2307 (missing module), TS7006 (implicit any), TS2366 (missing return) — all in files either unmodified by this PR or from pre-existing code (e.g., getTodoClass/getTodoIcon from PR Feat/daemon react cli #4380).
  3. 1 test failureslashCompletionSource > completes implicit /mcp subcommands — a test logic issue in the completion source that predates this PR. Verified failing identically with base branch code.

PR-Specific Verification

Check Result
/mode command removed from localCommands.ts ✅ Verified
New ApprovalModeMessage component compiles and renders ✅ Verified (lib build passes)
customization.tsx exports (hiddenCommands, toolCardHeader, markdownProcessor) ✅ Verified in lib build
Prettier on all PR files ✅ All formatted
ESLint on all PR files ✅ 0 errors
i18n.tsx new keys ✅ Present
Dialog CSS changes (DialogPrimitives.module.css) ✅ Compiles
ToolGroup.tsx customization hook integration ✅ Compiles

Conclusion

PR is merge-ready. The lib build (which is the distributable output), ESLint, Prettier, and 92 unit tests all pass. The 3 failing items are pre-existing on the base branch and unrelated to this PR's changes.


Verified locally by wenshao

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ytahdn ytahdn merged commit c97d1b9 into QwenLM:daemon_mode_b_main Jun 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants