diff --git a/docs/design/2026-11-04-channel-editor-progressive-disclosure.md b/docs/design/2026-11-04-channel-editor-progressive-disclosure.md new file mode 100644 index 00000000000..0231b4c142c --- /dev/null +++ b/docs/design/2026-11-04-channel-editor-progressive-disclosure.md @@ -0,0 +1,218 @@ +# Channel Editor Progressive Disclosure + +[English](2026-11-04-channel-editor-progressive-disclosure.md) | [简体中文](2026-11-04-channel-editor-progressive-disclosure.zh-CN.md) + +Status: proposed (2026-11-04), revised after review round 1 + +## Problem + +The Web Shell channel editor (`ChannelEditorDialog.tsx`, shared by all seven +platforms) renders every descriptor field flat. For GitHub a fresh instance +shows roughly 16 controls, each with a description paragraph: + +- Low-frequency fields (`pollInterval`, `reasonFilter`, `messageRoutes`, + `defaultMessageRoute`, `multiSession`, `instructions`) sit at the same level + as required credentials. +- `reasonFilter` alone renders 15 always-expanded checkbox pills. + +The dialog body scrolls well past the viewport, and the fields an operator +actually needs on first setup (name, workspace, authentication, endpoints, +group policy) are buried in noise. + +## Current state + +- Sections are already key-set driven: `CONNECTION_FIELD_KEYS`, + `SHARED_ACCESS_FIELD_KEYS`, `SHARED_SESSION_FIELD_KEYS` in + `packages/web-shell/client/components/channels/ChannelEditorDialog.tsx`. +- Save and validation iterate **all** descriptor fields regardless of what is + rendered: `buildChannelUpsertRequest` and `validateChannelEditorDraft` in + `channel-editor-state.ts` read `draft.values` for every field. Hiding a + field therefore changes neither what is saved nor what is validated. +- Conditional visibility has established precedents in the same dialog: + `allowedUsers` only under allowlist/pairing, `groupAllowedUsers` only under + an allowlist senders policy, per-repository rows only under an allowlist + group policy. +- One hard runtime dependency is relevant here: `defaultMessageRoute` must + name a configured route (`packages/channels/base/src/ChannelBase.ts`, + constructor throws + `defaultMessageRoute must name a configured message route.`). +- The `prWatchEvents` / `deliverToBoundSessions` dependency is addressed + separately by PR #16236, which keeps both fields visible and expresses the + dependency with disabled controls, an inline hint, and client-side + validation. This design deliberately leaves the visibility of those two + fields untouched so the two changes stay complementary. +- The unified `radix-ui` package (already a web-shell dependency) exports a + `Collapsible` primitive; every existing `ui/` primitive imports from + `radix-ui`. No `collapsible.tsx` wrapper existed before this change. + +## Goals + +- Shrink the default GitHub editor to the fields needed for first setup. +- Keep every configured value visible or one click away, and never hide a + validation error. +- Change no save/validation semantics and no descriptor/protocol surface. + +## Scope boundaries + +- Web Shell only: `ChannelEditorDialog.tsx`, its CSS module, the new + `ui/collapsible.tsx` primitive, `channel-editor-state.ts` (one shared record + predicate), `i18n.tsx`, collocated tests, and the visuals screenshot + scenarios. +- No changes to `packages/cli`, `packages/core`, `packages/channels`, or the + SDK types. +- Out of scope: the watch-events dependency UX (PR #16236), compact + multi-select for `reasonFilter`, description copy slimming, tabs/wizard + restructuring. These remain possible follow-ups. + +## Proposed solution + +### 1. Conditional default message route + +- `defaultMessageRoute` renders only when `messageRoutes` holds at least one + non-empty route **or** `defaultMessageRoute` itself is non-empty. The second + disjunct keeps an invalid stored value visible and fixable instead of + hiding the only control that could repair it. "Holds a route" uses the same + shape `assignField` writes: a JSON record with at least one non-empty string + value. + +### 2. Advanced settings collapsible + +- A new `ADVANCED_FIELD_KEYS` set, following the existing key-set pattern: + `pollInterval`, `reasonFilter`, `messageRoutes`, `defaultMessageRoute`, + `multiSession`, `instructions`. +- Deliberate exclusions: + - `baseUrl` and `wsUrl` are primary endpoints: they decide where a token is + sent (GitHub Enterprise / self-hosted GitLab, WeCom). A fresh draft must + show them, so they stay in Connection. Neither is declared `required` by + its descriptor; the exclusion is about being the platform's primary + endpoint, not about the required flag. + - `operators` is a permission grant ("who may approve tool use"), which + belongs in Access control next to `privatePolicy` and `allowedUsers`, not + behind a disclosure labelled as tuning. This is a deliberate product + decision, not a consequence of the key set. + - Only non-enum keys belong in the set: `initialFieldValue` pre-fills enums + from their default, so an enum key would count every fresh draft as + configured and auto-open the panel, quietly undoing the slim default + view. A comment on the set records this trap. +- The existing field partitions (credentials/connection, session, access) + each exclude advanced keys. Advanced fields render in one collapsible panel + at the end of the form, after access control, in descriptor order. The + former `routing` section became unreachable once every routing key moved to + the panel, so its scaffolding (`ROUTING_FIELD_KEYS`, the section-map entry, + the `fieldSectionKey` branch) is removed rather than left dead. +- The panel applies to every platform because the editor is shared; GitHub + benefits most. DingTalk keeps `outputMode` in Conversation management. +- Open/closed state: + - On dialog open, the panel starts **open** when any advanced field is + configured: booleans are `true`, records hold a non-empty route, other + kinds hold a non-empty trimmed string. Otherwise it starts collapsed. + - A collapsed panel shows a count badge of configured advanced fields so + nothing is silently hidden. + - Submitting with a validation error on any advanced key forces the panel + open **and scrolls the first erroring advanced field into view** inside + the dialog body (the only scroll container), because opening the panel + alone can leave the error below the fold. +- Radix `Collapsible` unmounts its content while closed. Draft values live in + the dialog's state, so collapsing never discards an edit, and validation + and serialization continue to cover every field. +- Accessibility: the panel title is a real `

` wrapping the trigger button + (WAI-ARIA accordion pattern). ARIA makes a button's descendants + presentational, so a span-in-button title would be invisible to heading + navigation; the label and badge stay inside the button so its accessible + name is unchanged. + +### 3. Shared record predicate + +- `recordHasRoute` / `parseRouteRecord` in `channel-editor-state.ts` are the + single source of the record rule ("a JSON record with at least one + non-empty string value"). Validation (`isMissingField`), the save path + (`assignField`), and the editor's visibility/auto-open predicates all read + it, so the three cannot drift apart. The dialog keeps only a thin + draft-value adapter (`draftHasRoute`). + +### 4. Primitive and copy + +- Add `packages/web-shell/client/components/ui/collapsible.tsx` in the house + style (shadcn structure, `import { Collapsible as CollapsiblePrimitive } +from 'radix-ui'`, `data-slot` attributes, `React.forwardRef` on all three + wrappers for the repository's React 18/19 ref contract, pinned by + `react18-ref-compat.test.tsx`). No new dependency, no portal, so no + portal-root integration is needed. +- New i18n keys in both locale maps: + - `channels.editor.section.advanced` — "Advanced settings" / "高级设置" + - `channels.editor.section.advanced.description` — one line explaining the + panel holds rarely changed tuning. + - `channels.editor.section.advanced.configuredCount` — parameterized count + badge, e.g. "2 configured" / "已配置 2 项". +- Existing field labels and descriptions are untouched. + +## Design decisions and rationale + +| Decision | Rationale | +| ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Key-set partition, not a descriptor flag | Matches the existing section mechanism; no protocol change, no CLI/core gate involvement. | +| Defer the watch-events dependency to PR #16236 | That open PR already expresses the dependency with disabled controls and client-side validation; hiding the same fields here would be a competing, conflicting UX. | +| `baseUrl` / `wsUrl` stay in Connection | They are primary endpoints that decide where a token is sent; burying them lets a fresh enterprise channel save without its endpoint and send the token to the public API. | +| `operators` stays in Access control | It grants permissions; discoverability of "who may approve tool use" outranks saving one row in the default view. Deliberate product decision. | +| `defaultMessageRoute` stays visible when itself non-empty | A stored invalid value must remain repairable in the form. | +| Auto-open on configured values; force-open plus scroll on errors | Collapsing must never strand a value or an error out of sight; opening the panel alone can leave the error below the fold. | +| Heading wraps the trigger | ARIA makes button descendants presentational; only a real heading outside the button is reachable by heading navigation. | +| One shared record predicate | Validation, save, and visibility agreed by coincidence before; one exported rule keeps them agreeing. | +| Advanced panel last, single panel | Least disruptive placement; one disclosure control instead of several. | + +## Constraints and risks + +- Existing tests pin section placement; they are updated to the new placement + (expand the panel first) while keeping the assertions meaningful. The + bound-session label tests are untouched because watch-event visibility does + not change here. +- Other platforms lose the routing section from the default view + (`messageRoutes`, `defaultMessageRoute` move to advanced). This is intended + and consistent; auto-open protects configured channels. +- The count badge treats an explicitly typed default (e.g. `pollInterval` + `60000`) as configured. Acceptable: it differs from "unset", which is what + the badge reports. +- The default view is slightly taller than the first revision because + Connection (with `baseUrl`) stays visible; correctness for enterprise + endpoints outranks the extra row. + +## Validation plan + +- Unit tests (collocated, vitest + jsdom): + - `defaultMessageRoute` hidden with no routes, shown when routes exist or + when it holds its own value. + - Advanced panel collapsed on a fresh GitHub draft with `baseUrl` still in + Connection; configured instances open it, with separate cases pinning the + string, boolean, and record arms of the configured predicate; the badge + count is asserted when collapsed. + - The panel title is an `

` wrapping the trigger button. + - A validation error on a collapsed advanced field forces the panel open on + submit and scrolls the erroring control into view (scrollIntoView stub + witness). + - Save payload parity: collapsed advanced values still serialize unchanged. + - Primary endpoints (`baseUrl`, `wsUrl`) stay in Connection while tuning + collapses; zh-CN copy for the new keys; updated section-placement tests. + - Mutation witnesses: each new mechanism (endpoint placement, heading, + boolean arm, record arm, scroll) has a mutation that turns exactly its + test red. +- `npm run build`, `npm run typecheck`, lint, and the focused web-shell test + files from `packages/web-shell`. +- Visuals e2e harness: before/after screenshots (dark and light) of the GitHub + editor with anchored assertions on the collapsed panel and its heading; the + repository-settings scenario anchors the configured-instance auto-open and + the count badge in a real browser. + +## Acceptance criteria + +- A fresh GitHub editor shows Identity, Credentials, Connection (including + `baseUrl`), Conversation isolation, Access control (including `operators`), + and a collapsed Advanced settings panel whose title is a heading. +- Configured advanced values open the panel on edit; validation errors inside + it force it open on submit and scroll the erroring field into view. +- Save/validation semantics are unchanged. +- All focused tests, typecheck, build, and lint pass. + +## Open questions + +None blocking. Badge semantics (count of non-empty advanced fields) are fixed +by this design. diff --git a/docs/design/2026-11-04-channel-editor-progressive-disclosure.zh-CN.md b/docs/design/2026-11-04-channel-editor-progressive-disclosure.zh-CN.md new file mode 100644 index 00000000000..b464693fa36 --- /dev/null +++ b/docs/design/2026-11-04-channel-editor-progressive-disclosure.zh-CN.md @@ -0,0 +1,115 @@ +# 渠道编辑器渐进式披露 + +[English](2026-11-04-channel-editor-progressive-disclosure.md) | [简体中文](2026-11-04-channel-editor-progressive-disclosure.zh-CN.md) + +状态：提案（2026-11-04），评审第 1 轮后修订 + +## 问题 + +Web Shell 渠道编辑器（`ChannelEditorDialog.tsx`，七个平台共用）把描述符里的所有字段平铺渲染。GitHub 新建实例大约显示 16 个控件，每个字段还带一段描述： + +- 低频字段（`pollInterval`、`reasonFilter`、`messageRoutes`、`defaultMessageRoute`、`multiSession`、`instructions`）与必填凭据处于同一层级。 +- 仅 `reasonFilter` 一项就渲染 15 个始终展开的复选框 pill。 + +对话框正文远超视口高度，首次配置真正需要的字段（名称、工作区、认证、端点、群组策略）被淹没在噪音里。 + +## 现状 + +- 分区已经由 key 集合驱动：`packages/web-shell/client/components/channels/ChannelEditorDialog.tsx` 中的 `CONNECTION_FIELD_KEYS`、`SHARED_ACCESS_FIELD_KEYS`、`SHARED_SESSION_FIELD_KEYS`。 +- 保存与校验遍历**全部**描述符字段，与是否渲染无关：`channel-editor-state.ts` 的 `buildChannelUpsertRequest` 和 `validateChannelEditorDraft` 对每个字段读取 `draft.values`。因此隐藏字段既不改变保存内容，也不改变校验范围。 +- 同一对话框已有条件显示先例：`allowedUsers` 仅在 allowlist/pairing 下显示，`groupAllowedUsers` 仅在 senders 为 allowlist 时显示，逐仓库行仅在群组策略为 allowlist 时显示。 +- 与本设计相关的一条硬运行时依赖：`defaultMessageRoute` 必须指向已配置的路由（`packages/channels/base/src/ChannelBase.ts`，构造函数抛出 `defaultMessageRoute must name a configured message route.`）。 +- `prWatchEvents` / `deliverToBoundSessions` 的依赖由 PR #16236 单独处理：它保留两个字段可见，用禁用控件、内联提示和客户端校验表达依赖。本设计刻意不动这两个字段的可见性，使两个改动互补。 +- 统一 `radix-ui` 包（已是 web-shell 依赖）导出 `Collapsible` 原语；现有 `ui/` 原语全部从 `radix-ui` 导入。本改动之前没有 `collapsible.tsx` 包装。 + +## 目标 + +- 把 GitHub 编辑器的默认视图收缩到首次配置所需的字段。 +- 每个已配置值要么可见、要么一次点击可达，且绝不隐藏校验错误。 +- 不改变保存/校验语义，不改变描述符/协议表面。 + +## 范围边界 + +- 仅 Web Shell：`ChannelEditorDialog.tsx`、其 CSS module、新增 `ui/collapsible.tsx` 原语、`channel-editor-state.ts`（一个共享 record 判定）、`i18n.tsx`、同址测试，以及 visuals 截图场景。 +- 不改 `packages/cli`、`packages/core`、`packages/channels` 或 SDK 类型。 +- 不在本轮范围：监视事件依赖 UX（PR #16236）、`reasonFilter` 紧凑多选、描述文案瘦身、tabs/向导式重构。这些保留为可能的后续工作。 + +## 方案 + +### 1. 条件显示默认消息路由 + +- `defaultMessageRoute` 仅在 `messageRoutes` 至少含一条非空路由**或**其自身非空时渲染。第二个析取项让无效的存储值保持可见、可修复，而不是藏起唯一能修复它的控件。“含路由”采用与 `assignField` 写入相同的形状：至少含一个非空字符串值的 JSON record。 + +### 2. 高级设置折叠区 + +- 新增 `ADVANCED_FIELD_KEYS` 集合，沿用现有 key 集合模式：`pollInterval`、`reasonFilter`、`messageRoutes`、`defaultMessageRoute`、`multiSession`、`instructions`。 +- 刻意排除： + - `baseUrl` 与 `wsUrl` 是主端点：它们决定 token 发往哪里（GitHub Enterprise / 自建 GitLab、WeCom）。全新草稿必须显示它们，因此留在 Connection。两者的描述符都没有声明 `required`；排除的理由是“平台主端点”，而不是 required 标志。 + - `operators` 是一处权限授予（“谁可以批准工具调用”），应与 `privatePolicy`、`allowedUsers` 一起留在访问控制，而不是藏在标着“调优项”的折叠区后面。这是刻意的产品决策，不是 key 集合带来的后果。 + - 集合里只放非 enum key：`initialFieldValue` 会用默认值预填 enum，因此一个 enum key 会让每个全新草稿都算作已配置并自动展开面板，悄悄抵消精简的默认视图。集合上的注释记录了这一陷阱。 +- 现有字段分区（凭据/连接、会话、访问）各自排除高级 key。高级字段按描述符顺序在表单末尾（访问控制之后）的一个折叠面板中渲染。原 `routing` 分区在所有路由 key 移入面板后不可达，其脚手架（`ROUTING_FIELD_KEYS`、分区映射项、`fieldSectionKey` 分支）被删除而不是留作死代码。 +- 面板作用于所有平台（编辑器共用）；GitHub 收益最大。DingTalk 的 `outputMode` 留在会话管理。 +- 展开/收起状态： + - 对话框打开时，若任一高级字段已配置则面板初始**展开**：布尔为 `true`、record 含非空路由、其余 kinds 含非空去空白字符串。否则初始收起。 + - 收起状态下面板显示已配置高级字段的数量徽标，确保没有东西被静默隐藏。 + - 提交时若任一高级 key 存在校验错误，先强制展开面板，**并把第一个出错的高级字段滚入视野**（对话框正文是唯一滚动容器）——只展开面板可能让错误仍在视口下方。 +- Radix `Collapsible` 收起时卸载内容。草稿值保存在对话框 state 中，收起不会丢弃任何编辑，校验与序列化继续覆盖每个字段。 +- 无障碍：面板标题是包裹触发按钮的真实 `

`（WAI-ARIA 手风琴写法）。ARIA 会让按钮的后代变成表现性内容，因此按钮内的 span 标题对标题导航不可见；标签与徽标仍留在按钮内，可访问名称不变。 + +### 3. 共享 record 判定 + +- `channel-editor-state.ts` 的 `recordHasRoute` / `parseRouteRecord` 是 record 规则（“至少含一个非空字符串值的 JSON record”）的唯一来源。校验（`isMissingField`）、保存路径（`assignField`）与编辑器的可见性/自动展开判定都读它，三者不会脱钩。对话框只保留一个薄薄的草稿值适配层（`draftHasRoute`）。 + +### 4. 原语与文案 + +- 按仓库风格新增 `packages/web-shell/client/components/ui/collapsible.tsx`（shadcn 结构，`import { Collapsible as CollapsiblePrimitive } from 'radix-ui'`，`data-slot` 属性；三个包装均用 `React.forwardRef`，满足仓库 React 18/19 ref 契约，由 `react18-ref-compat.test.tsx` 钉住）。无新依赖、无 portal，因此不需要 portal-root 集成。 +- 两个 locale map 各新增 i18n key： + - `channels.editor.section.advanced` — "Advanced settings" / "高级设置" + - `channels.editor.section.advanced.description` — 一句话说明面板存放少改的调优项。 + - `channels.editor.section.advanced.configuredCount` — 参数化数量徽标，如 "2 configured" / "已配置 2 项"。 +- 现有字段标签与描述不动。 + +## 设计决策与理由 + +| 决策 | 理由 | +| ---------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| key 集合分区，而非描述符标志 | 与现有分区机制一致；无协议变更，不涉及 CLI/core 门禁。 | +| 监视事件依赖让位 PR #16236 | 该 OPEN PR 已用禁用控件与客户端校验表达依赖；在此隐藏同一批字段会构成互斥、冲突的 UX。 | +| `baseUrl` / `wsUrl` 留在 Connection | 它们是决定 token 发往哪里的主端点；埋掉它们会让新建企业渠道在没有端点的情况下保存，把 token 发往公共 API。 | +| `operators` 留在访问控制 | 它授予权限；“谁可以批准工具调用”的可发现性比默认视图少一行更重要。刻意的产品决策。 | +| `defaultMessageRoute` 自身非空时保持可见 | 存储的无效值必须能在表单里修复。 | +| 已配置值自动展开；错误强制展开并滚动 | 折叠绝不能让值或错误脱离视线；只展开面板可能让错误仍在视口下方。 | +| 标题包裹触发器 | ARIA 让按钮后代变成表现性内容；只有按钮外的真实标题才能被标题导航到达。 | +| 一个共享 record 判定 | 校验、保存、可见性此前只是恰好一致；一个导出规则让它们继续一致。 | +| 高级面板置底、单一面板 | 干扰最小的位置；一个披露控件而非多个。 | + +## 约束与风险 + +- 现有测试固定了分区归属；将它们更新到新归属（先展开面板），同时保持断言有意义。bound-session 标签测试不动，因为监视事件可见性在此不变。 +- 其他平台默认视图失去路由分区（`messageRoutes`、`defaultMessageRoute` 移入高级）。这是预期且一致的；已配置渠道由自动展开保护。 +- 数量徽标把显式输入的默认值（如 `pollInterval` `60000`）视为已配置。可接受：它不同于“未设置”，而徽标报告的正是后者。 +- 默认视图比第一版略高，因为 Connection（含 `baseUrl`）保持可见；企业端点的正确性优先于少一行。 + +## 验证计划 + +- 单元测试（同址，vitest + jsdom）： + - `defaultMessageRoute` 无路由时隐藏、有路由或自身有值时显示。 + - 全新 GitHub 草稿下高级面板收起且 `baseUrl` 仍在 Connection；已配置实例打开时展开，字符串、布尔、record 三个判定分支各有独立用例钉住；收起时断言徽标计数。 + - 面板标题是包裹触发按钮的 `

`。 + - 收起的高级字段上的校验错误在提交时强制展开面板，并把出错控件滚入视野（scrollIntoView 桩见证）。 + - 保存载荷一致性：收起的高级值仍原样序列化。 + - 主端点（`baseUrl`、`wsUrl`）留在 Connection 而调优项折叠；新 key 的 zh-CN 文案；更新后的分区归属测试。 + - 变异见证：每个新机制（端点归属、标题、布尔分支、record 分支、滚动）都有一个恰好让其测试变红的变异。 +- 从 `packages/web-shell` 运行 `npm run build`、`npm run typecheck`、lint 及聚焦的 web-shell 测试文件。 +- Visuals e2e harness：按仓库 visuals 工作流，对 GitHub 编辑器出 before/after 截图（暗/亮各一），锚定收起面板及其标题；仓库设置场景在真实浏览器中锚定已配置实例的自动展开与数量徽标。 + +## 验收标准 + +- 全新 GitHub 编辑器显示 Identity、Credentials、Connection（含 `baseUrl`）、会话隔离、Access control（含 `operators`），以及标题为真实标题的收起 Advanced settings 面板。 +- 已配置的高级值在编辑时展开面板；其内的校验错误在提交时强制展开并把出错字段滚入视野。 +- 保存/校验语义不变。 +- 聚焦测试、typecheck、build、lint 全部通过。 + +## 开放问题 + +无阻塞项。徽标语义（非空高级字段计数）由本设计固定。 diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css b/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css index 417457cf894..882128f7ea5 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css @@ -90,6 +90,47 @@ line-height: 1.5; } +.advancedTrigger { + display: flex; + width: 100%; + align-items: center; + gap: 8px; + padding: 0; + border: none; + background: none; + color: inherit; + font: inherit; + letter-spacing: inherit; + cursor: pointer; + text-align: left; +} + +.advancedChevron { + flex: 0 0 auto; + color: var(--muted-foreground); + transition: transform 120ms ease; +} + +.advancedTrigger[data-state='open'] .advancedChevron { + transform: rotate(180deg); +} + +.advancedBadge { + margin-left: auto; + padding: 2px 8px; + border: 1px solid var(--border); + border-radius: 999px; + color: var(--muted-foreground); + font-size: 11px; + font-weight: 500; +} + +.advancedContent { + display: flex; + flex-direction: column; + gap: 14px; +} + .field { display: flex; flex-direction: column; diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx index 8e3c4e57a79..02c34e3356f 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx @@ -427,6 +427,20 @@ async function chooseRadioOption(optionLabel: string) { }); } +// Advanced fields unmount while the panel is collapsed; tests that exercise +// them expand it first. A panel that auto-opened on configured values is left +// alone so the helper never toggles it shut. +async function expandAdvancedSettings(label = 'Advanced settings') { + const trigger = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.includes(label), + ); + expect(trigger).toBeDefined(); + if (trigger!.getAttribute('data-state') === 'open') return; + await act(async () => { + trigger!.click(); + }); +} + function setInputValue(input: HTMLInputElement, value: string) { Object.getOwnPropertyDescriptor( HTMLInputElement.prototype, @@ -649,6 +663,7 @@ describe('ChannelEditorDialog', () => { ], }; await renderDialog({ descriptor }); + await expandAdvancedSettings(); expect(document.body.textContent).toContain('Named tasks'); expect(document.body.textContent).toContain( @@ -762,6 +777,7 @@ describe('ChannelEditorDialog', () => { it('renders a multiline string field as a textarea and leaves a plain one as an input', async () => { await renderDialog({ descriptor: MULTILINE_INSTRUCTIONS }); + await expandAdvancedSettings(); const instructions = fieldByLabel('Instructions'); const endpoint = fieldByLabel('API endpoint (descriptor)'); @@ -770,15 +786,17 @@ describe('ChannelEditorDialog', () => { expect(endpoint?.tagName).toBe('INPUT'); }); - it('groups the shared instructions control with conversation management', async () => { + it('files the shared instructions control under advanced settings', async () => { await renderDialog({ descriptor: MULTILINE_INSTRUCTIONS }); + await expandAdvancedSettings(); - // `instructions` is not a credential: it is stored in clear text and - // injected into the session context, so it belongs with the other shared - // session controls rather than in the catch-all credentials panel. - expect(sectionHeadingOf(fieldByLabel('Instructions'))).toBe( - 'Conversation management', + // `instructions` is rarely changed tuning: it lives in the advanced + // disclosure panel rather than the catch-all credentials panel, while a + // plain credential string stays in Credentials. + const panel = fieldByLabel('Instructions')?.closest( + '[data-slot="collapsible"]', ); + expect(panel?.textContent).toContain('Advanced settings'); expect(sectionHeadingOf(fieldByLabel('API endpoint (descriptor)'))).toBe( 'Credentials', ); @@ -786,6 +804,7 @@ describe('ChannelEditorDialog', () => { it('renders the localized instructions copy above the textarea instead of the descriptor literal', async () => { await renderDialog({ descriptor: MULTILINE_INSTRUCTIONS }); + await expandAdvancedSettings(); // `instructions` is the only multiline field in the fixture, so the single // textarea anchors the field. fieldDescription resolves @@ -852,6 +871,7 @@ describe('ChannelEditorDialog', () => { descriptor: MULTILINE_INSTRUCTIONS, language: 'zh-CN', }); + await expandAdvancedSettings('高级设置'); // getTranslator resolves `messages[key] ?? EN[key] ?? key`, so an assertion // phrased only as "not the raw key", or as an English substring, still @@ -865,6 +885,7 @@ describe('ChannelEditorDialog', () => { it('saves a multi-line instructions value with the newline intact', async () => { const onSave = vi.fn().mockResolvedValue(undefined); await renderDialog({ descriptor: MULTILINE_INSTRUCTIONS, onSave }); + await expandAdvancedSettings(); const instructions = fieldByLabel('Instructions'); expect(instructions).toBeInstanceOf(HTMLTextAreaElement); @@ -1129,6 +1150,8 @@ describe('ChannelEditorDialog', () => { }; const onSave = vi.fn().mockResolvedValue(undefined); await renderDialog({ descriptor, onSave }); + // Operators is a permission grant: it renders in Access control, not in + // the advanced panel, so no expansion is needed here. await act(async () => { setInputValue(inputByLabel('Instance name')!, 'release-bot'); setInputValue(inputByLabel('Client ID')!, 'ding-client-id'); @@ -1272,6 +1295,7 @@ describe('ChannelEditorDialog', () => { it('renders the reason filter as checkboxes and saves the checked reasons', async () => { const onSave = vi.fn().mockResolvedValue(undefined); await renderDialog({ descriptor: GITHUB_CONFIGURED, onSave }); + await expandAdvancedSettings(); const group = document.querySelector( '[role="group"][aria-label="Reason Filter"]', @@ -1611,13 +1635,16 @@ describe('ChannelEditorDialog', () => { expect(onSave).not.toHaveBeenCalled(); }); - it('separates connection and notification fields from credentials', async () => { + it('files connection and notification tuning under advanced settings', async () => { await renderDialog({ descriptor: GITHUB_CONFIGURED }); + await expandAdvancedSettings(); - expect(sectionHeadingOf(fieldByLabel('Base URL'))).toBe('Connection'); - expect(sectionHeadingOf(fieldByLabel('Reason Filter'))).toBe( - 'Notifications and routing', + // The endpoint stays in Connection; only the notification tuning moves. + expect(sectionHeadingOf(inputByLabel('Base URL'))).toBe('Connection'); + const panel = fieldByLabel('Reason Filter')?.closest( + '[data-slot="collapsible"]', ); + expect(panel?.textContent).toContain('Advanced settings'); expect(sectionHeadingOf(inputByLabel('Personal Access Token'))).toBe( 'Credentials', ); @@ -1626,17 +1653,28 @@ describe('ChannelEditorDialog', () => { ); }); - it('splits connection and routing fields for a non-GitHub channel', async () => { - await renderDialog({ descriptor: GITLAB_ROUTING }); + it('files connection and routing tuning under advanced settings for a non-GitHub channel', async () => { + // A stored default route keeps its field visible even without routes, so + // the fixture can assert the whole routing group inside the panel. + const instance: DaemonChannelInstanceSnapshot = { + name: 'gitlab-bot', + config: { type: 'gitlab', defaultMessageRoute: '/qa' }, + secrets: {}, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITLAB_ROUTING, instance }); + await expandAdvancedSettings(); - expect(sectionHeadingOfLabel('Instance URL')).toBe('Connection'); expect(sectionHeadingOfLabel('Personal Access Token')).toBe('Credentials'); - expect(sectionHeadingOfLabel('Message Routes')).toBe( - 'Notifications and routing', - ); - expect(sectionHeadingOfLabel('Default Message Route')).toBe( - 'Notifications and routing', - ); + expect(sectionHeadingOfLabel('Instance URL')).toBe('Connection'); + const panelOf = (labelText: string) => + Array.from(document.querySelectorAll('label')) + .find((item) => item.textContent?.includes(labelText)) + ?.closest('[data-slot="collapsible"]'); + const panel = panelOf('Message Routes'); + expect(panel?.textContent).toContain('Advanced settings'); + expect(panelOf('Default Message Route')).toBe(panel); }); it('asks for repository full names when GitHub groups are allowlisted', async () => { @@ -2051,15 +2089,19 @@ describe('GitHub per-repository settings', () => { } }); - it('renders the poll interval under Connection', async () => { + it('renders the poll interval under advanced settings', async () => { await renderDialog({ descriptor: GITHUB_POLLING }); + await expandAdvancedSettings(); - expect(sectionHeadingOfLabel('Poll Interval (ms)')).toBe('Connection'); + const panel = Array.from(document.querySelectorAll('label')) + .find((item) => item.textContent?.includes('Poll Interval (ms)')) + ?.closest('[data-slot="collapsible"]'); + expect(panel?.textContent).toContain('Advanced settings'); expect(inputByLabel('Poll Interval (ms)')?.type).toBe('number'); }); - it('files the poll interval under Connection for any platform declaring it', async () => { - // The section map is keyed by field name, so Email's existing interval + it('files the poll interval under advanced settings for any platform declaring it', async () => { + // The advanced map is keyed by field name, so Email's existing interval // moves too. Pin the documented side effect instead of leaving it to prose. const email: DaemonChannelTypeDescriptor = { type: 'email', @@ -2076,8 +2118,12 @@ describe('GitHub per-repository settings', () => { ], }; await renderDialog({ descriptor: email }); + await expandAdvancedSettings(); - expect(sectionHeadingOfLabel('Poll Interval (ms)')).toBe('Connection'); + const panel = Array.from(document.querySelectorAll('label')) + .find((item) => item.textContent?.includes('Poll Interval (ms)')) + ?.closest('[data-slot="collapsible"]'); + expect(panel?.textContent).toContain('Advanced settings'); expect(document.body.textContent).not.toContain('Per-repository settings'); }); @@ -2187,9 +2233,13 @@ describe('GitHub per-repository settings', () => { // The fixture's descriptor label is byte-identical to the English string, // so only a zh-CN render proves the label mapping is load-bearing. await renderDialog({ descriptor: GITHUB_POLLING, language: 'zh-CN' }); + await expandAdvancedSettings('高级设置'); expect(inputByLabel('轮询间隔（毫秒）')).not.toBeNull(); - expect(sectionHeadingOfLabel('轮询间隔（毫秒）')).toBe('连接'); + const panel = Array.from(document.querySelectorAll('label')) + .find((item) => item.textContent?.includes('轮询间隔（毫秒）')) + ?.closest('[data-slot="collapsible"]'); + expect(panel?.textContent).toContain('高级设置'); }); it('names the repositories whose settings outlive the allowlist policy', async () => { @@ -2510,6 +2560,7 @@ describe('GitHub per-repository settings', () => { it('names both bounds when the descriptor declares both', async () => { await renderDialog({ descriptor: GITHUB_POLLING }); + await expandAdvancedSettings(); await act(async () => { setInputValue(inputByLabel('Poll Interval (ms)')!, '2592000000'); }); @@ -2520,3 +2571,329 @@ describe('GitHub per-repository settings', () => { ); }); }); + +describe('Channel editor progressive disclosure', () => { + function advancedTrigger(): HTMLButtonElement | undefined { + return Array.from(document.querySelectorAll('button')).find((button) => + button.textContent?.includes('Advanced settings'), + ); + } + + async function clickAdvancedTrigger() { + const trigger = advancedTrigger(); + expect(trigger).toBeDefined(); + await act(async () => { + trigger!.click(); + }); + } + + async function saveDialog() { + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + } + + it('hides the default message route until a route exists', async () => { + await renderDialog({ descriptor: GITLAB_ROUTING }); + await expandAdvancedSettings(); + + // Record fields render a label without a control id, so assert on the + // label text rather than fieldByLabel. + const labels = Array.from(document.querySelectorAll('label')).map( + (label) => label.textContent ?? '', + ); + expect(labels.some((text) => text.includes('Message Routes'))).toBe(true); + expect(labels.some((text) => text.includes('Default Message Route'))).toBe( + false, + ); + }); + + it('shows the default message route when routes are configured', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'gitlab-bot', + config: { + type: 'gitlab', + messageRoutes: { '/qa': 'Answer questions' }, + }, + secrets: {}, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITLAB_ROUTING, instance }); + await expandAdvancedSettings(); + + expect(fieldByLabel('Default Message Route')).not.toBeNull(); + }); + + it('collapses advanced settings on a fresh draft', async () => { + await renderDialog({ descriptor: GITHUB_CONFIGURED }); + + expect(advancedTrigger()?.getAttribute('data-state')).toBe('closed'); + // The primary endpoint stays visible; only tuning collapses. + expect(sectionHeadingOf(inputByLabel('Base URL'))).toBe('Connection'); + expect(fieldByLabel('Reason Filter')).toBeNull(); + // No count badge while nothing is configured (the word also occurs in + // unrelated field copy, so scope the check to the trigger). + expect(advancedTrigger()?.textContent).not.toContain('configured'); + + await clickAdvancedTrigger(); + expect(advancedTrigger()?.getAttribute('data-state')).toBe('open'); + expect(fieldByLabel('Reason Filter')).not.toBeNull(); + }); + + it('renders the advanced panel title as a heading outside the trigger', async () => { + await renderDialog({ descriptor: GITHUB_CONFIGURED }); + + // ARIA makes a button's descendants presentational, so the section title + // must be a real heading wrapping the trigger, not a span inside it. + const heading = Array.from(document.querySelectorAll('h3')).find((h) => + h.textContent?.includes('Advanced settings'), + ); + expect(heading).toBeDefined(); + const trigger = advancedTrigger(); + expect(trigger?.tagName).toBe('BUTTON'); + expect(heading!.contains(trigger!)).toBe(true); + }); + + it('opens advanced settings for configured values and badges the count when collapsed', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + ...GITHUB_CONFIGURED, + fields: [ + ...GITHUB_CONFIGURED.fields, + { + key: 'instructions', + label: 'Instructions', + kind: 'string', + multiline: true, + }, + ], + }; + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + groupPolicy: 'open', + reasonFilter: ['mention'], + instructions: 'Review carefully', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor, instance }); + + expect(advancedTrigger()?.getAttribute('data-state')).toBe('open'); + expect(fieldByLabel('Instructions')).toHaveProperty( + 'value', + 'Review carefully', + ); + + await clickAdvancedTrigger(); + expect(advancedTrigger()?.getAttribute('data-state')).toBe('closed'); + expect(advancedTrigger()?.textContent).toContain('2 configured'); + }); + + it('opens advanced settings for a stored boolean value', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + ...GITHUB_CONFIGURED, + fields: [ + ...GITHUB_CONFIGURED.fields, + { key: 'multiSession', label: 'Named Sessions', kind: 'boolean' }, + ], + }; + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { type: 'github', groupPolicy: 'open', multiSession: true }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor, instance }); + + // Pins the boolean arm of the configured predicate: without it the + // stored value would open collapsed out of sight. + expect(advancedTrigger()?.getAttribute('data-state')).toBe('open'); + }); + + it('opens advanced settings for stored message routes', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'gitlab-bot', + config: { + type: 'gitlab', + messageRoutes: { '/qa': 'Answer questions' }, + }, + secrets: {}, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITLAB_ROUTING, instance }); + + // Pins the record arm of the configured predicate, without the tolerant + // expand helper so the auto-open itself is what passes the test. + expect(advancedTrigger()?.getAttribute('data-state')).toBe('open'); + }); + + it('forces advanced settings open and scrolls the erroring field into view', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + ...GITHUB_CONFIGURED, + fields: [ + ...GITHUB_CONFIGURED.fields, + { + key: 'pollInterval', + label: 'Poll Interval (ms)', + kind: 'number', + exclusiveMinimum: 0, + maximum: 86_400_000, + }, + ], + }; + const onSave = vi.fn().mockResolvedValue(undefined); + // jsdom has no scrollIntoView; stub it so the scroll is observable. + const scrollIntoView = vi.fn(); + const originalScrollIntoView = Element.prototype.scrollIntoView; + Element.prototype.scrollIntoView = scrollIntoView; + try { + await renderDialog({ descriptor, onSave }); + await expandAdvancedSettings(); + await act(async () => { + setInputValue(inputByLabel('Poll Interval (ms)')!, '2592000000'); + }); + await clickAdvancedTrigger(); + expect(advancedTrigger()?.getAttribute('data-state')).toBe('closed'); + + await saveDialog(); + + expect(advancedTrigger()?.getAttribute('data-state')).toBe('open'); + expect(document.body.textContent).toContain( + 'Enter a number greater than 0 and at most 86400000.', + ); + expect(onSave).not.toHaveBeenCalled(); + // Opening the panel is not enough: the error must be brought into + // view, on the erroring control itself. + expect(scrollIntoView).toHaveBeenCalledWith({ block: 'center' }); + expect(scrollIntoView.mock.instances[0]).toBe( + inputByLabel('Poll Interval (ms)'), + ); + } finally { + Element.prototype.scrollIntoView = originalScrollIntoView; + } + }); + + it('saves collapsed advanced values unchanged', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + ...GITHUB_CONFIGURED, + fields: [ + ...GITHUB_CONFIGURED.fields, + { + key: 'instructions', + label: 'Instructions', + kind: 'string', + multiline: true, + }, + ], + }; + const onSave = vi.fn().mockResolvedValue(undefined); + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + groupPolicy: 'open', + reasonFilter: ['mention', 'comment'], + instructions: 'Review carefully', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor, instance, onSave }); + await clickAdvancedTrigger(); + expect(advancedTrigger()?.getAttribute('data-state')).toBe('closed'); + + await saveDialog(); + + expect(onSave).toHaveBeenCalledWith( + 'github-bot', + expect.objectContaining({ + config: expect.objectContaining({ + reasonFilter: ['mention', 'comment'], + instructions: 'Review carefully', + }), + }), + ); + }); + + it('keeps primary endpoints in Connection while tuning moves to advanced', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + type: 'wecom', + displayName: 'WeCom', + manageable: true, + fields: [ + { key: 'botId', label: 'Bot ID', kind: 'string', required: true }, + { key: 'wsUrl', label: 'WebSocket URL', kind: 'string' }, + { key: 'baseUrl', label: 'Base URL', kind: 'string' }, + { + key: 'pollInterval', + label: 'Poll Interval (ms)', + kind: 'number', + exclusiveMinimum: 0, + }, + ], + }; + await renderDialog({ descriptor }); + await expandAdvancedSettings(); + + // Both endpoints decide where a token is sent, so a fresh draft must + // show them; only genuine tuning collapses. + expect(sectionHeadingOf(fieldByLabel('WebSocket URL'))).toBe('Connection'); + expect(sectionHeadingOf(fieldByLabel('Base URL'))).toBe('Connection'); + const panel = fieldByLabel('Poll Interval (ms)')?.closest( + '[data-slot="collapsible"]', + ); + expect(panel?.textContent).toContain('Advanced settings'); + }); + + it('localizes the advanced panel and its count badge for zh-CN', async () => { + const descriptor: DaemonChannelTypeDescriptor = { + ...GITHUB_CONFIGURED, + fields: [ + ...GITHUB_CONFIGURED.fields, + { + key: 'instructions', + label: 'Instructions', + kind: 'string', + multiline: true, + }, + ], + }; + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + groupPolicy: 'open', + reasonFilter: ['mention'], + instructions: 'Review carefully', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ + descriptor, + instance, + language: 'zh-CN', + }); + + const trigger = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.includes('高级设置'), + ); + expect(trigger).toBeDefined(); + expect(trigger?.getAttribute('data-state')).toBe('open'); + await act(async () => { + trigger!.click(); + }); + expect(trigger?.textContent).toContain('已配置 2 项'); + }); +}); diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx index 70751ecfc80..6535043593a 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx @@ -12,7 +12,7 @@ import { type FormEvent, type ReactNode, } from 'react'; -import { CheckCircle2Icon, KeyRoundIcon } from 'lucide-react'; +import { CheckCircle2Icon, ChevronDownIcon, KeyRoundIcon } from 'lucide-react'; import type { DaemonChannelConfigFieldDescriptor, DaemonChannelInstanceSnapshot, @@ -30,6 +30,11 @@ import { extractErrorDetail } from '../../utils/errorDetail'; import { workspaceLabel } from '../../utils/workspace'; import { Alert, AlertDescription, AlertTitle } from '../ui/alert'; import { Button } from '../ui/button'; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from '../ui/collapsible'; import { Dialog, DialogContent, @@ -63,6 +68,7 @@ import { PRIVATE_ONLY_FIELD_KEYS, invalidRepositoryGroupEntries, projectRepositoryGroup, + recordHasRoute, repositoryAllowlistIds, retainedRepositorySettings, toList, @@ -146,10 +152,23 @@ const FIELD_PLACEHOLDER_KEYS: Record = { }; const CONNECTION_FIELD_KEYS = new Set(['baseUrl', 'wsUrl', 'pollInterval']); -const ROUTING_FIELD_KEYS = new Set([ +/** + * Rarely changed tuning, collapsed into one disclosure panel. Primary + * endpoints stay out: `wsUrl` (WeCom) and `baseUrl` (GitHub Enterprise / + * self-hosted GitLab) decide where a token is sent, so a fresh draft must + * show them. `operators` stays out too: it grants permissions, which belongs + * in Access control, not behind a tuning disclosure. Only non-enum keys + * belong here — `initialFieldValue` pre-fills enums from their default, so + * an enum key would count every fresh draft as configured and auto-open the + * panel, quietly undoing the slim default view. + */ +const ADVANCED_FIELD_KEYS = new Set([ + 'pollInterval', 'reasonFilter', 'messageRoutes', 'defaultMessageRoute', + 'multiSession', + 'instructions', ]); /** Every repository starts fully inheriting; nothing is materialized. */ @@ -161,9 +180,8 @@ const EMPTY_GROUP_OVERRIDE: ChannelGroupOverrideDraft = { function fieldSectionKey( field: DaemonChannelConfigFieldDescriptor, -): 'credentials' | 'connection' | 'routing' { +): 'credentials' | 'connection' { if (CONNECTION_FIELD_KEYS.has(field.key)) return 'connection'; - if (ROUTING_FIELD_KEYS.has(field.key)) return 'routing'; return 'credentials'; } @@ -210,6 +228,20 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +/** Draft-value adapter over the shared record rule in channel-editor-state. */ +function draftHasRoute(value: string | boolean | undefined): boolean { + return typeof value === 'string' && recordHasRoute(value); +} + +function isAdvancedFieldConfigured( + field: DaemonChannelConfigFieldDescriptor, + value: string | boolean | undefined, +): boolean { + if (field.kind === 'boolean') return value === true; + if (field.kind === 'record') return draftHasRoute(value); + return typeof value === 'string' && value.trim() !== ''; +} + function FieldShell({ id, label, @@ -280,14 +312,23 @@ export function ChannelEditorDialog({ const [submitError, setSubmitError] = useState(); const [saving, setSaving] = useState(false); const [reloading, setReloading] = useState(false); + const [advancedOpen, setAdvancedOpen] = useState(false); + const [scrollToErrorKey, setScrollToErrorKey] = useState(); const dismissedRef = useRef(false); /** The token draft as it stood before local gh auth overrode it. */ const tokenBeforeLocalGh = useRef(undefined); - const accessFields = descriptor.fields.filter((field) => - SHARED_ACCESS_FIELD_KEYS.has(field.key), + const accessFields = descriptor.fields.filter( + (field) => + SHARED_ACCESS_FIELD_KEYS.has(field.key) && + !ADVANCED_FIELD_KEYS.has(field.key), ); - const sessionFields = descriptor.fields.filter((field) => - SHARED_SESSION_FIELD_KEYS.has(field.key), + const sessionFields = descriptor.fields.filter( + (field) => + SHARED_SESSION_FIELD_KEYS.has(field.key) && + !ADVANCED_FIELD_KEYS.has(field.key), + ); + const advancedFields = descriptor.fields.filter((field) => + ADVANCED_FIELD_KEYS.has(field.key), ); const sessionScopeField = sessionFields.find( (field) => field.key === 'sessionScope' && field.kind === 'enum', @@ -306,14 +347,24 @@ export function ChannelEditorDialog({ const credentialFields = descriptor.fields.filter( (field) => !SHARED_ACCESS_FIELD_KEYS.has(field.key) && - !SHARED_SESSION_FIELD_KEYS.has(field.key), + !SHARED_SESSION_FIELD_KEYS.has(field.key) && + !ADVANCED_FIELD_KEYS.has(field.key), ); useEffect(() => { if (!open) return; dismissedRef.current = false; tokenBeforeLocalGh.current = undefined; - setDraft(createChannelEditorDraft(descriptor, instance)); + const fresh = createChannelEditorDraft(descriptor, instance); + setDraft(fresh); + // Configured tuning must not hide behind a collapsed panel on open. + setAdvancedOpen( + descriptor.fields.some( + (field) => + ADVANCED_FIELD_KEYS.has(field.key) && + isAdvancedFieldConfigured(field, fresh.values[field.key]), + ), + ); setErrors({}); setSubmitError(undefined); }, [descriptor, instance, open]); @@ -323,6 +374,31 @@ export function ChannelEditorDialog({ setSubmitError(undefined); }, [workspaceCwd]); + useEffect(() => { + if (!advancedOpen || !scrollToErrorKey) return; + // Runs after the forced-open panel content mounts; the id is the same one + // FieldShell renders, so the scroll lands on the erroring control. The + // optional call keeps jsdom (no scrollIntoView) from throwing. + document + .getElementById(`${formId}-${scrollToErrorKey}`) + ?.scrollIntoView?.({ block: 'center' }); + setScrollToErrorKey(undefined); + }, [advancedOpen, scrollToErrorKey, formId]); + + const isFieldVisible = (field: DaemonChannelConfigFieldDescriptor) => { + // A stored default route must stay repairable even with no routes left. + if (field.key === 'defaultMessageRoute') + return ( + draftHasRoute(draft.values['messageRoutes']) || + String(draft.values['defaultMessageRoute'] ?? '').trim() !== '' + ); + return true; + }; + + const advancedConfiguredCount = advancedFields.filter((field) => + isAdvancedFieldConfigured(field, draft.values[field.key]), + ).length; + const fieldLabel = (field: DaemonChannelConfigFieldDescriptor) => { const key = FIELD_LABEL_KEYS[descriptor.type]?.[field.key] ?? @@ -412,6 +488,15 @@ export function ChannelEditorDialog({ instance, ); if (Object.keys(validation).length > 0) { + // An error on a collapsed field would otherwise be invisible: open the + // panel and bring the first erroring advanced field into view. + const advancedErrorKey = Object.keys(validation).find((key) => + ADVANCED_FIELD_KEYS.has(key), + ); + if (advancedErrorKey) { + setAdvancedOpen(true); + setScrollToErrorKey(advancedErrorKey); + } setErrors( Object.fromEntries( Object.entries(validation).map(([key, code]) => [ @@ -1186,22 +1271,20 @@ export function ChannelEditorDialog({ - {(['credentials', 'connection', 'routing'] as const).map( - (section) => { - const sectionFields = credentialFields.filter( - (field) => fieldSectionKey(field) === section, - ); - if (sectionFields.length === 0) return null; - return ( -
-

- {t(`channels.editor.section.${section}`)} -

- {sectionFields.map(renderField)} -
- ); - }, - )} + {(['credentials', 'connection'] as const).map((section) => { + const sectionFields = credentialFields.filter( + (field) => fieldSectionKey(field) === section, + ); + if (sectionFields.length === 0) return null; + return ( +
+

+ {t(`channels.editor.section.${section}`)} +

+ {sectionFields.map(renderField)} +
+ ); + })} {sessionFields.length > 0 ? (
@@ -1508,6 +1591,41 @@ export function ChannelEditorDialog({
); })()} + + {advancedFields.length > 0 ? ( + + {/* Heading outside the trigger: ARIA makes a button's + descendants presentational, so a span-in-button title + would be invisible to heading navigation. */} +

+ + +

+ +

+ {t('channels.editor.section.advanced.description')} +

+ {advancedFields.filter(isFieldVisible).map(renderField)} + + + ) : null}