Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

全屏长文本编辑器有两份实现:form.tsx 的 FullscreenTextarea 与 fields 的 FullscreenFieldEditor(合并受阻于 components ← fields 的依赖方向) #3398

Closed

Description

@yinlianghui

观察类记录(finding,不带 pm:queue)。在实施 #3303(PR #3397,删掉内置 textarea 分支的无生产者 fullscreen 别名)时确认。

#3303 正文里记过这条,但那单即将随 PR #3397 关闭,这条观察会被埋进已关闭的 issue 里;PM 当时的裁定是「不做,依赖方向反,需单独裁决」,那就需要一个自己的载体来承接这次裁决。单独开一条,不带 pm:queue,等 triage。

现象

同一个「全屏编辑长文本」的交互(展开按钮 + 全高对话框 + draft/commit 语义),当前有两份独立实现:

实现 位置 消费者
FullscreenTextarea packages/components/src/renderers/form/form.tsx:357(origin/main @ 4eeb932) 表单渲染器的内置(未注册)textarea 分支
FullscreenFieldEditor packages/fields/src/widgets/FullscreenFieldEditor.tsx:95(160 行) TextAreaField.tsx:85、RichTextField.tsx:145

#3302(issue #3301)已经把 widget 侧原本的两份合并成了共享的 FullscreenFieldEditor,内置分支这一份没有跟进 —— 不是遗漏,是搬不过去(见下)。

为什么这不是一次简单搬运

依赖方向是反的,实测自 package.json:

  • packages/fields/package.json 依赖 "@object-ui/components": "workspace:*";
  • packages/components/package.json 的 dependencies / peerDependencies 里没有 @object-ui/fields。

即 fields → components。所以 components/src/renderers/form/form.tsx 不能 import FullscreenFieldEditor。AGENTS.md §3 的拓扑也是这么排的(Atoms 在下、Fields 在上)。

为什么今天不算 bug(观察类的理由)

两份实现目前行为一致,用户碰不到差异;#3272(PR #3392)刚把 FullscreenTextarea 的文案接进 i18n,两边都是活的、都被测试覆盖。它是漂移风险,不是现存缺陷 —— 但 #3301 那一单的教训正是「一个表单级开关产出两种行为、且没有任何东西报告这个漂移」,两份状态机长期并存就是同一形状的种子。

一个已经发生的具体例子:#3393(全屏对话框拿不到字段 label)只落在 FullscreenTextarea 这一份上;同类问题要不要在 FullscreenFieldEditor 上再修一遍,取决于这两份最终是否合并。

可能的方向(需裁决,本 issue 不预设结论)

  • A. 把共享实现下沉到 packages/components,fields 的两个 widget 改为从下层 import。顺依赖方向,一份实现;代价是 components(定位「Pure UI, no business logic」)要接纳一个带 draft/commit 状态机的编辑器外壳,且要确认它不拖进 fields 侧的依赖。
  • B. 维持两份,显式记为「两个渲染路径各自拥有」,并加一条对齐测试防漂移。成本最低,但把重复固化了。
  • C. 让内置 textarea 分支干脆不再自带全屏实现(例如该分支本身就走注册 widget)。最彻底,但改动面远超本条观察,需要单独评估内置分支的存在理由。

倾向不在此下:这条取决于 packages/components 的职责边界怎么划,属于架构裁决,不是实现细节。

证据等级

静态证据:读 form.tsx / FullscreenFieldEditor.tsx / 两个 widget 的调用点,以及两个 package.json 的依赖字段。未做浏览器复现(两份实现当前行为一致,无可复现的用户可见差异 —— 这正是它属于观察类的原因)。

关联:#3301(PR #3302 合并了 widget 侧两份)、#3303(PR #3397)、#3393、#3272(PR #3392)。

Activity

  1. yinlianghui commented on Aug 5, 2026

    @yinlianghui
    ContributorAuthor

    交叉引用:本条正文里「两份实现目前行为一致,用户碰不到差异」这句已经被证伪,详见刚开的 #3400(实测,非静态推断)。

    差异点是 readonly / disabled:

    • 注册路径 TextAreaField.tsx:40 在 readonly 时提前返回只读展示,全屏按钮根本不渲染;
    • 内置分支 FullscreenTextarea 两个状态都不认 —— 只读长文本连内联控件都能直接改(readonly 在 renderFieldComponent 入口就被解构走了,全屏出口没转发),禁用字段虽然内联控件是灰的,但展开按钮可点、对话框可改、「完成」把值写回表单状态。

    对本条的影响只有一处:它现在不再只是漂移风险,漂移已经发生了,而且是用户可见的。选项 B(维持两份 + 加对齐测试)的成本因此要重估 —— 对齐测试的第一条用例就是这个,而且得先把内置分支修到与注册路径一致才谈得上「对齐」。另外 FullscreenFieldEditor 自己 grep readonly|readOnly|disabled 零命中,它只是被 TextAreaField 的提前返回挡着;所以选项 A(下沉合并)要把这两个状态的语义在合并时定义出来,不能假设搬过去就自带。

    裁决本身仍归本条,#3400 只记缺陷。


    Generated by Claude Code

  2. yinlianghui commented on Aug 6, 2026

    @yinlianghui
    ContributorAuthor

    发现分诊轮(持有):前提 live(双全屏编辑器并存),但具体漂移(#3400)已单修并有 pin;合并本体需先裁 packages/components 边界方向(A/B/C),与 #3439 同属「内建 form 分支 parity」一揽子裁决,待维护者。(objectui 分片 PM,会话 session_01GTRjn8xBqp75dk7kFupVRt)


    Generated by Claude Code

  3. claude commented on Aug 7, 2026

    @claude
    Contributor

    Findings triage (15:47Z fire) — HOLD confirmed, premise re-verified.

    Re-checked against origin/main: both implementations are still present and independent — FullscreenTextarea in packages/components/src/renderers/form/form.tsx, FullscreenFieldEditor in packages/fields/src/widgets/. The hold's stated basis also verifies: the concrete drift (#3400) is fixed and pinned, and the pin is real — form-fullscreen-textarea-readonly-disabled.test.tsx now sits alongside the i18n and label pins in the same directory. So the residual question is unchanged and is still a boundary ruling, not a merge chore: option A requires packages/components ("Pure UI, no business logic") to take on a draft/commit state machine, and option B's alignment-test cost was already re-estimated upward by #3400. Hold stands, still bundled with the #3439 built-in-form-branch parity ruling.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. os-zhuang commented on Aug 9, 2026

    @os-zhuang
    Contributor

    Findings triage (re-verification round) — HOLD confirmed, finding retained.

    Premise re-checked on origin/main @ cdc0e44: both implementations still present and independent — FullscreenTextarea at packages/components/src/renderers/form/form.tsx:432 (moved from :357; re-anchor from there) and FullscreenFieldEditor at packages/fields/src/widgets/FullscreenFieldEditor.tsx:181. The blocking question is unchanged: the A/B/C boundary ruling for packages/components ("Pure UI, no business logic" vs a draft/commit state machine), still bundled with #3439's built-in-form-branch parity ruling, still un-adjudicated. No restart condition has fired.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  5. os-zhuang commented on Aug 10, 2026

    @os-zhuang
    Contributor

    Finding-triage ruling: Escalate — finding to needs-user-decision.

    Two implementations of the fullscreen text editor (FullscreenTextarea in components at form.tsx:432, FullscreenFieldEditor in fields at :181) already drifted once — #3400, fixed and pinned. The merge is blocked purely on the dependency direction between components and fields, and all four holds on this card plus #3439's end at the same sentence: awaits the maintainer's boundary ruling. Holding further only accrues re-verification cost.

    Question for the maintainer: pick the boundary — A allow the dependency and merge into one implementation, B lift the shared editor into a lower package both can import, or C bless the duplication with a parity pin. #3439 (built-in textarea character count) reprices automatically with this answer.

    Authorization: maintainer directive (this session) —「对于issue中的findings 执行一次集中分诊,并更新issue 的状态。」Centralized pass under that directive; finding grading is normally the triage seat's single channel. Round: objectstack#4949 finding-triage, 2026-08-10.
    Session: 01JaVVMrSxt7Tgi1uwEuDtH7


    Generated by Claude Code

  6. os-zhuang commented on Aug 10, 2026

    @os-zhuang
    Contributor

    Maintainer ruling (2026-08-10, directed in session session_01BPWqbmEFU8gJepBJTHESXd): one implementation, hoisted to the dependency-legal side.

    Hoist the shared fullscreen-editor primitive into the package both sides may depend on (components), and make the fields-side FullscreenFieldEditor a thin wrapper over it. If the measured import graph makes that direction impossible, report the graph on this card before merging the other way — do not resolve the dependency block by keeping two implementations.

    needs-user-decision → pm:queue.


    Generated by Claude Code

  7. self-assigned this
    on Aug 10, 2026
  8. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    ContributorAuthor

    Claim: PM loop round 6 (rolling slot refill; maintainer-ruled card — ruling of 2026-08-10 05:12 in the comment above)
    Session: session_017Qqyix2QcnpUC9XeYVDzx3 · Branch: claude/issue-3398-fullscreen-editor-hoist · Worktree: objectui-issue-3398
    File surface: packages/components (the hoisted primitive + form.tsx's FullscreenTextarea retirement onto it) + packages/fields (FullscreenFieldEditor → thin wrapper; TextAreaField/RichTextField untouched unless the wrapper's contract forces it) + tests both sides. Serial constraints: disjoint from #4167/#2627/#4178/#4181 in flight (#2627 is AI chat, different components area).
    Container & model: M, mode:subagent, model: opus

    Binding context beyond the ruling text: the #3400 lesson is load-bearing — FullscreenFieldEditor greps zero for readonly/disabled (shielded only by TextAreaField's early return), while the components side carries the #3400 fix and its pin (form-fullscreen-textarea-readonly-disabled.test.tsx). The hoisted primitive must therefore DEFINE the readonly/disabled semantics explicitly (the merged single implementation owns them; both call paths inherit them), and the #3400 pin must keep passing against the merged code. The escape hatch is verbatim from the ruling: if the measured import graph blocks the hoist, REPORT THE GRAPH on the card before merging the other way. #3439 (character-count parity) reprices with this card but is NOT in scope.


    Generated by Claude Code

  9. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    ContributorAuthor

    ACCEPT (objectui seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3, review of record) — PR #4193.

    Executed per the maintainer's ruling (hoist to @object-ui/components), and the ruling's legality was measured before the first edit: fields → components is the existing dependency direction and components names fields nowhere, so the escape hatch stayed shut. The primitive now defines readonly/disabled rather than inheriting them — readOnly renders no affordance, disabled leaves an inert one, gated at open / injected editor / Done / onCommit (covering the isSubmitting flip mid-dialog) — and the #3400 pin passes unmodified against the merged code, which is the whole point of the card.

    Two corrections to the dispatch's binding context, both surfaced rather than worked around, both accepted: (1) the "fields side greps zero for readonly/disabled" claim had expired — #3402/#3404 already hardened that copy, so this was a union of two hardened implementations, with the readonly half of the lesson still live and load-bearing; (2) toggleClassName has zero producers repo-wide and was never exported from the fields barrel, so it was correctly NOT carried onto the new public export — minting a producer-less prop on a fresh public API is exactly the #3232/#3233 shape.

    Three reverse probes, all red in the predicted direction: readonly gate deletion → #3400 pin red; wrapper dropping disabled → 9/21 red including the write-back gate; toggleClassName+readOnly paste → TS2322, proving tsc read the rebuilt cross-package .d.ts. Full suites, 31-consumer type-check sweep, i18n byte-drift zero, CI converged 20/20 (18 success + 2 path-filter skips). Changeset components:minor + fields:patch accepted. Flipping ready + auto-merge.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions