refactor(components,fields): one fullscreen long-text editor, hoisted to components (#3398) - #4193
Merged
Merged
Conversation
… to components (#3398) The "expand to a full-height dialog" interaction had two independent implementations: `FullscreenTextarea` inside the form renderer's built-in `textarea` branch (@object-ui/components) and `FullscreenFieldEditor` in @object-ui/fields for the registered TextAreaField / RichTextField widgets. One form-level promise (ObjectFormSchema.mobile.fullscreenLongText) is honoured on two render paths, and each path grew its own answer. They drifted in both directions — #3400 (readonly editable through the built-in dialog), #3402 (the same write-back hole for disabled on the registered path), #3393 and #3272 each landing on one side first. Per the maintainer ruling of 2026-08-10, the shared primitive is hoisted to the dependency-legal side. Measured import graph: fields depends on components; components declares no dependency on fields in dependencies or peerDependencies. So components now exports `FullscreenEditor` and `FullscreenFieldEditor` becomes a thin wrapper over it. The primitive DEFINES readOnly/disabled rather than inheriting them by accident: readOnly renders no affordance at all, disabled leaves an inert one, and both are gated at open, in the injected editor, on Done, and at onCommit — because disabled also carries isSubmitting and can flip while the dialog is open. Copy is unchanged: the same form.fullscreen.* / common.cancel keys through createSafeTranslation with byte-identical English defaults, so no locale pack changes and provider-less hosts render what they did. The now-unread form.fullscreen.* defaults are dropped from useFieldTranslation. `toggleClassName` is not carried into the new primitive — zero producers repo-wide, and FullscreenFieldEditor is not exported from the fields barrel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 10, 2026 19:45
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3398
One fullscreen long-text editor, hoisted to the package both render paths may import.
The measured import graph (the ruling's escape hatch, answered)
The maintainer's ruling of 2026-08-10 requires the hoist direction be checked against the real graph before any code moves, so this was measured first, from the
package.jsonfiles on this tip:packages/fields/package.json→dependenciescontains"@object-ui/components": "workspace:*".packages/components/package.json→ neitherdependenciesnorpeerDependenciesmentions@object-ui/fields.So the edge is
fields → components, one-way, and the ruled direction is legal. The escape hatch is not taken and no second implementation survives.Ruling text, quoted verbatim and untranslated:
What changed
@object-ui/componentsgainsFullscreenEditor(src/custom/fullscreen-editor.tsx), a single primitive owning the expand affordance, the dialog, the draft/commit state machine and the copy. The editor itself stays injected throughchildren, so nothing widget-specific moved down.form.tsx'sFullscreenTextareakeeps only what is its own — the inlineTextareaand the editor it injects — and renders the primitive for everything else. ItsDialogfamily andMaximize2/Check/Ximports went with the deleted copy.packages/fields'FullscreenFieldEditorbecomes a thin wrapper. Same name, same props, same test-id namespaces, so both hosts and every existing pin are untouched.form.fullscreen.*defaults are dropped fromuseFieldTranslation.common.cancelstays —RecordPickerDialogandPeoplePickerread it independently.The load-bearing half: readonly/disabled are DEFINED, not inherited
Per the PM's binding context, this is the #3400 lesson applied to the merge. Neither copy defined both states: the components one grew them under #3400, while the fields one declared only
disabledand was shielded fromreadonlyby its hosts' early return. A single implementation cannot be shielded by one caller's control flow, so the primitive answers both and both call paths inherit the same answers:readOnly→ no affordance at all (it means "shown plainly"; a disabled button advertises an affordance the read-only path does not have).disabled→ the toggle stays but is inert (it means "not interactive, muted").Neither leans on the toggle alone, because
disabledalso carries the form'sisSubmittingand can flip true while the dialog is already open: opening refuses independently of the attribute, the injected editor is told, "Done" is disabled, andonCommitis gated as the single point where a value leaves for host state.The wrapper keeps
readOnlyOUT of the fields contract —Omit< FullscreenEditorProps, 'readOnly' >— because both fields hosts early-return a read-only display before rendering it, so the prop would have no producer on that path (the #3232/#3233 shape). That is why the hoist did not simply re-export the primitive.Behaviour and copy parity
No copy changed and no locale pack needed an edit. The primitive consumes the same
form.fullscreen.*/common.cancelkeys both copies already read, throughcreateSafeTranslationwith English defaults byte-identical to the literals, so provider-less hosts render exactly what they did.check:i18n-driftconfirms 0 en values changed;check:i18n-keysresolves every call site.toggleClassNameis not carried into the new primitive: zero producers repo-wide, andFullscreenFieldEditoris not exported from the@object-ui/fieldsbarrel, so nothing outside the package could ever set it. Publishing it as part of a NEW public export incomponentswould have minted a prop with no producer.#3439 (built-in textarea character count) is out of scope and unchanged.
Verification
All commands run from the repo root per AGENTS.md.
Targeted pins —
pnpm exec vitest run --maxWorkers=2over the four components fullscreen test files:Fields-side fullscreen tests (8 files incl. both no-provider/i18n pins):
Full suites —
pnpm exec vitest run --maxWorkers=2 packages/components/ packages/fields/:packages/plugin-form/(the built-in branch's consumer):Downstream consumer type-check sweep —
pnpm --workspace-concurrency=2 --filter '...@object-ui/components' --filter '...@object-ui/fields' type-check. Direction stated explicitly: the PREFIX form, i.e. the 31 consumer packages, not upstream deps. All 31 Done, after a fullpackages/**build so no result rode a stale or missingdist.Lint: components 0 errors / 881 warnings, fields 0 errors / 748 warnings — all warnings pre-existing.
check:control-bytes,check:i18n-keys,check:i18n-drift,check-changeset-presence,check-changeset-no-majorall green.Reverse verification (three, all in the predicted direction)
if (readOnly) return null;) → the form.tsx 内置 textarea 的全屏路径完全绕过 readonly / disabled：只读长文本可直接编辑，禁用字段可经对话框改值并提交进表单状态 #3400 pin goes RED, 2 failed / 12 passed, failing onexpect(expandButton()).toBeNull()— the expand button reappears on a read-only field, exactly the defect form.tsx 内置 textarea 的全屏路径完全绕过 readonly / disabled：只读长文本可直接编辑，禁用字段可经对话框改值并提交进表单状态 #3400 closed. Restored.disabledon its way through → 9 failed / 12 passed acrossTextAreaField.fullscreenDisabledandRichTextField.fullscreenDisabled, including the write-back gate itself (expected "vi.fn()" to not be called at all, but actually been called 1 times). Restored.toggleClassNameandreadOnlyonto the fields call site is REJECTED —error TS2322 … not assignable to type 'IntrinsicAttributes & FullscreenFieldEditorProps'— which is what proves tsc read the rebuilt@object-ui/components.d.tsrather than a cached one. Restored.Generated by Claude Code