feat(web-shell): collapse channel editor tuning behind advanced settings (#16239)

* feat(web-shell): collapse channel editor tuning behind advanced settings

The shared channel editor renders every descriptor field flat, so a fresh GitHub instance shows roughly sixteen controls and the fields needed for first setup are buried under rarely changed tuning. Move the tuning fields into one collapsible advanced panel and hide the default message route until a route exists, keeping the default view to identity, credentials, conversation isolation, and access control.

Collapsing never hides state: the panel opens automatically when any advanced field is configured, shows a count badge while collapsed, and forces itself open when a collapsed field fails validation. Save and validation semantics are unchanged because both already iterate every descriptor field regardless of rendering. The watch-events dependency UX is left to #16236 so the two changes stay complementary.

* fix(web-shell): forward refs through the collapsible wrappers

Review of #16239 showed the three new wrappers accept Radix props including ref, but plain function components drop external refs on React 18, and the repository supports both React 18 and 19. Use React.forwardRef for the root, trigger, and content, pass each ref to its Radix primitive, and pin the contract in react18-ref-compat.test.tsx with both forward-ref type and DOM-ref assertions; a plain-function mutation turns exactly the three new type assertions red. Record the contract in both design documents as well.

* fix(web-shell): keep endpoints and operators visible, make collapsed errors reachable

Review round 1 on #16239 found the disclosure went too far in three places and left dead scaffolding behind. Primary endpoints (baseUrl for GitHub Enterprise / self-hosted GitLab, wsUrl for WeCom) decide where a token is sent, so they stay in Connection instead of collapsing; a fresh enterprise channel could otherwise save without its endpoint and send the token to the public API. Operators is a permission grant and stays in Access control, a deliberate product decision rather than a consequence of the key set.

The panel title becomes a real h3 wrapping the trigger (ARIA makes button descendants presentational, so the previous span-in-button was invisible to heading navigation), and a rejected submit now scrolls the first erroring advanced field into view instead of only opening the panel below the fold. The record rule is unified into one exported predicate shared by validation, the save path, and the visibility/auto-open readers; the unreachable routing section scaffolding is removed; a comment records why enum keys must stay out of the advanced set.

Each new mechanism carries a mutation witness (endpoint placement, heading, boolean arm, record arm, scroll), and the visuals scenarios now anchor the heading, the configured-instance auto-open, and the count badge in a real browser. Both design documents are revised in lockstep.
This commit is contained in:
Shaojin Wen authored and GitHub committed 2026-11-04 16:46:45 +08:00
1 parent 30079d6c3d
commit 815c1d3768
10 files changed
+1072 -81

No files matched your search

@@ -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 `<h3>` 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 `<h3>` 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.
@@ -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 中,收起不会丢弃任何编辑,校验与序列化继续覆盖每个字段。
- 无障碍:面板标题是包裹触发按钮的真实 `<h3>`(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 三个判定分支各有独立用例钉住;收起时断言徽标计数。
- 面板标题是包裹触发按钮的 `<h3>`。
- 收起的高级字段上的校验错误在提交时强制展开面板,并把出错控件滚入视野(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 全部通过。
## 开放问题
无阻塞项。徽标语义(非空高级字段计数)由本设计固定。
@@ -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;
@@ -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 项');
});
});
@@ -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<string, string> = {
};
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<string, unknown> {
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<string>();
const [saving, setSaving] = useState(false);
const [reloading, setReloading] = useState(false);
const [advancedOpen, setAdvancedOpen] = useState(false);
const [scrollToErrorKey, setScrollToErrorKey] = useState<string>();
const dismissedRef = useRef(false);
/** The token draft as it stood before local gh auth overrode it. */
const tokenBeforeLocalGh = useRef<ChannelSecretDraft | undefined>(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({
</FieldShell>
</section>
{(['credentials', 'connection', 'routing'] as const).map(
(section) => {
const sectionFields = credentialFields.filter(
(field) => fieldSectionKey(field) === section,
);
if (sectionFields.length === 0) return null;
return (
<section key={section} className={styles.section}>
<h3 className={styles.sectionHeading}>
{t(`channels.editor.section.${section}`)}
</h3>
{sectionFields.map(renderField)}
</section>
);
},
)}
{(['credentials', 'connection'] as const).map((section) => {
const sectionFields = credentialFields.filter(
(field) => fieldSectionKey(field) === section,
);
if (sectionFields.length === 0) return null;
return (
<section key={section} className={styles.section}>
<h3 className={styles.sectionHeading}>
{t(`channels.editor.section.${section}`)}
</h3>
{sectionFields.map(renderField)}
</section>
);
})}
{sessionFields.length > 0 ? (
<section className={styles.settingsPanel}>
@@ -1508,6 +1591,41 @@ export function ChannelEditorDialog({
</section>
);
})()}
{advancedFields.length > 0 ? (
<Collapsible
open={advancedOpen}
onOpenChange={setAdvancedOpen}
className={styles.settingsPanel}
>
{/* Heading outside the trigger: ARIA makes a button's
descendants presentational, so a span-in-button title
would be invisible to heading navigation. */}
<h3 className={styles.settingsPanelTitle}>
<CollapsibleTrigger className={styles.advancedTrigger}>
<ChevronDownIcon
size={15}
className={styles.advancedChevron}
aria-hidden="true"
/>
<span>{t('channels.editor.section.advanced')}</span>
{!advancedOpen && advancedConfiguredCount > 0 ? (
<span className={styles.advancedBadge}>
{t('channels.editor.section.advanced.configuredCount', {
count: advancedConfiguredCount,
})}
</span>
) : null}
</CollapsibleTrigger>
</h3>
<CollapsibleContent className={styles.advancedContent}>
<p className={styles.settingsPanelDescription}>
{t('channels.editor.section.advanced.description')}
</p>
{advancedFields.filter(isFieldVisible).map(renderField)}
</CollapsibleContent>
</Collapsible>
) : null}
</div>
<DialogFooter className="mt-4">
<Button
@@ -95,6 +95,36 @@ export function toList(value: string): string[] {
.filter(Boolean);
}
/**
* The one record rule, shared by validation, the save path, and the editor's
* visibility/auto-open predicates: a JSON record holding at least one
* non-empty string value. Returns the filtered record so the save path and
* the boolean readers cannot drift apart.
*/
export function parseRouteRecord(
value: string,
): Record<string, string> | undefined {
if (!value.trim()) return undefined;
try {
const parsed: unknown = JSON.parse(value);
if (!isRecord(parsed)) return undefined;
const filtered = Object.fromEntries(
Object.entries(parsed).filter(
([, entry]) => typeof entry === 'string' && entry.trim(),
),
);
return Object.keys(filtered).length > 0
? (filtered as Record<string, string>)
: undefined;
} catch {
return undefined;
}
}
export function recordHasRoute(value: string): boolean {
return parseRouteRecord(value) !== undefined;
}
export function hasDescriptorPrivatePolicy(
descriptor: DaemonChannelTypeDescriptor,
): boolean {
@@ -324,16 +354,7 @@ function isMissingField(
}
const value = draft.values[field.key];
if (field.kind === 'record') {
if (typeof value !== 'string' || !value.trim()) return true;
try {
const parsed: unknown = JSON.parse(value);
if (!isRecord(parsed)) return true;
return Object.values(parsed).every(
(v) => typeof v !== 'string' || !v.trim(),
);
} catch {
return true;
}
return typeof value !== 'string' || !recordHasRoute(value);
}
return typeof value === 'string' ? value.trim().length === 0 : false;
}
@@ -578,23 +599,10 @@ function assignField(
.map((s) => s.trim())
.filter(Boolean);
} else if (field.kind === 'record') {
try {
const parsed: unknown = JSON.parse(value);
if (!isRecord(parsed)) {
delete config[field.key];
return;
}
const filtered = Object.fromEntries(
Object.entries(parsed).filter(
([, v]) => typeof v === 'string' && v.trim(),
),
);
if (Object.keys(filtered).length > 0) {
config[field.key] = filtered;
} else {
delete config[field.key];
}
} catch {
const filtered = parseRouteRecord(value);
if (filtered) {
config[field.key] = filtered;
} else {
delete config[field.key];
}
} else {
@@ -0,0 +1,42 @@
'use client';
import { forwardRef } from 'react';
import type { ComponentPropsWithoutRef } from 'react';
import { Collapsible as CollapsiblePrimitive } from 'radix-ui';
const Collapsible = forwardRef<
HTMLDivElement,
ComponentPropsWithoutRef<typeof CollapsiblePrimitive.Root>
>(function Collapsible(props, ref) {
return (
<CollapsiblePrimitive.Root ref={ref} data-slot="collapsible" {...props} />
);
});
const CollapsibleTrigger = forwardRef<
HTMLButtonElement,
ComponentPropsWithoutRef<typeof CollapsiblePrimitive.Trigger>
>(function CollapsibleTrigger(props, ref) {
return (
<CollapsiblePrimitive.Trigger
ref={ref}
data-slot="collapsible-trigger"
{...props}
/>
);
});
const CollapsibleContent = forwardRef<
HTMLDivElement,
ComponentPropsWithoutRef<typeof CollapsiblePrimitive.Content>
>(function CollapsibleContent(props, ref) {
return (
<CollapsiblePrimitive.Content
ref={ref}
data-slot="collapsible-content"
{...props}
/>
);
});
export { Collapsible, CollapsibleTrigger, CollapsibleContent };
@@ -7,6 +7,11 @@ import { describe, expect, it } from 'vitest';
import { AlertDialogContent, AlertDialogOverlay } from './alert-dialog';
import { Button } from './button';
import { Checkbox } from './checkbox';
import {
Collapsible,
CollapsibleContent,
CollapsibleTrigger,
} from './collapsible';
import { DialogContent, DialogOverlay } from './dialog';
import { DrawerContent, DrawerOverlay } from './drawer';
import { DropdownMenuSubTrigger, DropdownMenuTrigger } from './dropdown-menu';
@@ -51,6 +56,9 @@ describe('React 18 ref compatibility', () => {
['AlertDialogOverlay', AlertDialogOverlay],
['Button', Button],
['Checkbox', Checkbox],
['Collapsible', Collapsible],
['CollapsibleContent', CollapsibleContent],
['CollapsibleTrigger', CollapsibleTrigger],
['DialogContent', DialogContent],
['DialogOverlay', DialogOverlay],
['DrawerContent', DrawerContent],
@@ -106,6 +114,32 @@ describe('React 18 ref compatibility', () => {
container.remove();
});
it('forwards Collapsible, trigger, and content refs to their DOM elements', () => {
const rootRef = React.createRef<HTMLDivElement>();
const triggerRef = React.createRef<HTMLButtonElement>();
const contentRef = React.createRef<HTMLDivElement>();
const container = document.createElement('div');
document.body.appendChild(container);
const root = createRoot(container);
// Content only mounts while open, so pin the open state explicitly.
act(() =>
root.render(
<Collapsible ref={rootRef} open>
<CollapsibleTrigger ref={triggerRef}>Toggle</CollapsibleTrigger>
<CollapsibleContent ref={contentRef}>Content</CollapsibleContent>
</Collapsible>,
),
);
expect(rootRef.current).toBeInstanceOf(HTMLDivElement);
expect(triggerRef.current).toBeInstanceOf(HTMLButtonElement);
expect(contentRef.current).toBeInstanceOf(HTMLDivElement);
expect(contentRef.current?.textContent).toBe('Content');
act(() => root.unmount());
container.remove();
});
it('forwards a Table ref to its DOM element', () => {
const ref = React.createRef<HTMLTableElement>();
const container = document.createElement('div');
@@ -1468,16 +1468,31 @@ for (const theme of THEMES) {
name: 'Use local GitHub CLI authentication',
}),
).toBeVisible();
// Anchor the capture to the checklist so a regression that falls back to
// a free-text filter fails here instead of shipping a stale picture.
// Anchor the slim default view: tuning lives behind the collapsed
// advanced panel, so a regression that flattens it back out fails here
// instead of shipping a stale picture.
const advancedTrigger = page.getByRole('button', {
name: 'Advanced settings',
});
await expect(advancedTrigger).toBeVisible();
await expect(advancedTrigger).toHaveAttribute('data-state', 'closed');
// The panel title must be reachable by heading navigation (a real h3
// wrapping the trigger, not a span inside the button).
await expect(
page.getByRole('heading', { name: 'Advanced settings' }),
).toBeVisible();
const reasonFilter = page.getByRole('group', { name: 'Reason Filter' });
await expect(reasonFilter).toHaveCount(0);
await captureScreenshot(page, `github-channel-editor-${theme}`);
// Expand the panel and anchor the capture to the checklist so a
// regression that falls back to a free-text filter fails here too.
await advancedTrigger.click();
await expect(reasonFilter.getByRole('checkbox')).toHaveCount(4);
await expect(
reasonFilter.getByRole('checkbox', {
name: /mention — the bot is @mentioned/,
}),
).toBeChecked({ checked: false });
await captureScreenshot(page, `github-channel-editor-${theme}`);
// The dialog body scrolls, and the checklist sits below the fold at this
// viewport. Capture it separately and pin that it is fully on screen, so
// the published picture cannot quietly omit the feature it documents.
@@ -1680,6 +1695,19 @@ for (const theme of THEMES) {
await expect(inheriting.getByLabel('Allowed speakers')).toHaveValue('');
await expect(page.getByLabel('Poll Interval (ms)')).toHaveValue('30000');
// Browser-side proof of the auto-open mitigation: this instance stores
// an advanced value, so the panel must open on edit, and collapsing it
// must report the configured count instead of hiding the value silently.
const advancedTrigger = page.getByRole('button', {
name: /Advanced settings/,
});
await expect(advancedTrigger).toHaveAttribute('data-state', 'open');
await advancedTrigger.click();
await expect(advancedTrigger).toHaveAttribute('data-state', 'closed');
await expect(advancedTrigger).toContainText('1 configured');
await advancedTrigger.click();
await expect(advancedTrigger).toHaveAttribute('data-state', 'open');
// Put the last row in frame, not just the section title: a capture can
// otherwise show the heading with every row below the fold. Only the last
// row is required to fit — at this viewport the two rows together are
+10
View File
@@ -4462,6 +4462,11 @@ const EN: Messages = {
'channels.editor.section.access': 'Access control',
'channels.editor.section.access.description':
'Configure direct-message and group access separately.',
'channels.editor.section.advanced': 'Advanced settings',
'channels.editor.section.advanced.description':
'Rarely changed tuning for connection, routing, and session behavior.',
'channels.editor.section.advanced.configuredCount': (v) =>
`${v?.count ?? 0} configured`,
'channels.editor.session.isolation': 'Conversation isolation',
'channels.editor.instanceName': 'Instance name',
'channels.editor.instanceNamePlaceholder': 'e.g. release-bot',
@@ -8946,6 +8951,11 @@ const ZH: Messages = {
'channels.editor.section.access': '访问控制',
'channels.editor.section.access.description':
'分别设置谁可以私聊,以及哪些群聊可以使用此频道。',
'channels.editor.section.advanced': '高级设置',
'channels.editor.section.advanced.description':
'连接、路由与会话行为的少改调优项。',
'channels.editor.section.advanced.configuredCount': (v) =>
`已配置 ${v?.count ?? 0} 项`,
'channels.editor.session.isolation': '会话隔离方式',
'channels.editor.instanceName': '实例名称',
'channels.editor.instanceNamePlaceholder': '例如 release-bot',