feat: plan + scaffold Lexical-backed VariableInput v2#674
Open
0xdeafcafe wants to merge 3 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR documents (ADR-0002 + Gherkin feature spec) and scaffolds a Lexical-backed VariableInputV2 implementation that lives alongside the legacy contenteditable input, with side-by-side rendering in the /lab/variable-input playground for parity verification.
Changes:
- Adds Lexical dependencies to
@beak/uiand introduces thevariable-input-v2/scaffold (editor component, chip node, plugins, ValueSections conversion, and a localStorage-backed preference hook). - Extends the shared legacy
VariableSelectorwith an optionalanchorRectto support caret-rect popover anchoring for Lexical. - Adds ADR-0002 and a comprehensive
variable-input.featureparity/behaviour contract, plus docs index updates.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks new Lexical (+ transitive) dependencies. |
| packages/ui/package.json | Adds lexical and @lexical/react to @beak/ui. |
| packages/ui/src/features/variable-input/playground/VariableInputPlayground.tsx | Renders legacy + V2 side-by-side; adds persisted “prefer V2” toggle. |
| packages/ui/src/features/variable-input/components/molecules/VariableSelector.tsx | Adds anchorRect prop and positioning branch for caret-rect anchoring. |
| packages/ui/src/features/variable-input-v2/index.ts | Exports VariableInputV2 + feature-flag helpers. |
| packages/ui/src/features/variable-input-v2/components/VariableInputV2.tsx | Lexical-backed VariableInput implementation + plugins + VariableEditor integration. |
| packages/ui/src/features/variable-input-v2/nodes/VariableChipNode.tsx | Lexical DecoratorNode implementing the .bvs-blob DOM contract and import/export. |
| packages/ui/src/features/variable-input-v2/plugins/VariableTriggerPlugin.tsx | Implements { trigger detection + VariableSelector integration and chip insertion. |
| packages/ui/src/features/variable-input-v2/plugins/SingleLinePlugin.tsx | Enforces single-line behaviour (Enter/newline/paste transforms). |
| packages/ui/src/features/variable-input-v2/plugins/InitialValuePlugin.tsx | One-shot initial seed from ValueSections on mount. |
| packages/ui/src/features/variable-input-v2/plugins/ExternalValueSyncPlugin.tsx | Debounced external prop sync with legacy-like 100ms guard. |
| packages/ui/src/features/variable-input-v2/plugins/ChipDataIndexPlugin.tsx | Stamps legacy data-index to reuse existing VariableEditor wiring during scaffold phase. |
| packages/ui/src/features/variable-input-v2/utils/value-sections-conversion.ts | ValueSections ↔ Lexical tree conversion helpers. |
| packages/ui/src/features/variable-input-v2/utils/feature-flag.ts | localStorage-backed preference hook for opting into V2 at call sites. |
| docs/features/variable-input.feature | Behaviour contract (caret, trigger, modal, clipboard, masking, sync, parity, etc.). |
| docs/features/README.md | Adds feature doc index entry for variable input. |
| docs/adr/README.md | Adds ADR-0002 to ADR index. |
| docs/adr/0002-lexical-variable-input.md | ADR describing Lexical decision, architecture, and cutover checklist. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Plans the replacement of the bespoke contenteditable VariableInput with a Lexical-backed editor: library choice, chip DOM contract reuse, ValueSections bridge, hard-cutover policy, and the vitest + Playwright test gate. Companion .feature covers caret, trigger, state-modal, clipboard, masking, missing variables, disabled/readOnly, single-line constraint, debounced sync, and the parity contract for every legacy call site.
Adds `packages/ui/src/features/variable-input-v2/` — a Lexical-backed
VariableInput implementation that lives next to the legacy contenteditable
one for the side-by-side phase described in ADR-0002. The legacy
implementation is untouched except for one backwards-compatible addition
to VariableSelector (an optional `anchorRect` prop the V2 trigger plugin
uses for popover positioning) and the playground, which now renders both
implementations stacked for direct comparison.
The chip rendering reuses the existing `.bvs-blob` DOM contract and CSS,
so chips look identical between the two. `ValueSections` is unchanged —
two boundary helpers translate it to/from Lexical's editor state. The
trigger flow (`{` opens the selector), the state-modal popover, and
masking all work; clipboard round-trip and adjacent-chip caret behaviour
still need work (called out in the spec).
A localStorage feature flag (`beak.featureFlags.variableInputV2`) is the
toggle for the playground — production call sites remain on the legacy
implementation, per the hard-cutover policy. The cutover PR is the
follow-up that retires the legacy folder and routes every call site at
once.
See docs/adr/0002-lexical-variable-input.md (committed earlier in 21ef603d)
and docs/features/variable-input.feature.
- VariableSelector: refire position effect on partIndex/offset
changes so the popover tracks the caret as the user types `{query}`.
Previously `Boolean(sel)` was effectively a one-shot dep.
- VariableChipNode: createDOM returns `<span>` instead of `<div>` so
the inline decorator is valid inside Lexical's host `<p>` element
and the browser doesn't auto-close the paragraph + re-parent the
chip. CSS already uses `display: inline-block`, so visuals are
identical.
- SingleLinePlugin: doc comment now describes how `Cmd/Ctrl + Enter`
is actually wired — VariableInputV2's own keydown listener on the
root element handles it and stops propagation, the global shortcut
layer never sees the event.
- feature-flag: dispatch the change event even when `localStorage`
throws (private mode, quota), so in-memory subscribers update and
UI toggles reflect the click. Persistence is best-effort.
b0e4faa to
2c44650
Compare
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
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.
Summary
Plans and scaffolds the replacement of the bespoke contenteditable
VariableInputwith a Lexical-backed implementation, per ADR-0002. Landsthe architecture decision + Gherkin behaviour spec, and the V2 scaffold
living side-by-side with legacy. The V2 implementation is NOT routed to
any production call site yet — only the in-app playground (Cmd+K →
"Variable input lab") mounts it, stacked against the legacy input for
direct comparison.
The cutover — deleting the legacy folder, renaming v2 → canonical path,
wiring every call site, and landing the test gate — is a follow-up PR.
ADR-0002 §5 has the ordered checklist.
Changes
docs/adr/0002-lexical-variable-input.mdwith 5sub-decisions (library choice, chip DOM reuse, ValueSections boundary,
trigger flow, hard-cutover policy) and
docs/features/variable-input.featurewith 32 scenarios covering caret, trigger, state modal, clipboard,
masking, missing variables, disabled/readOnly, single-line constraint,
parity contract, and debounced sync. Both READMEs updated.
packages/ui/src/features/variable-input-v2/:VariableInputV2,VariableChipNode(a LexicalDecoratorNodereusingthe same
.bvs-blobDOM contract as legacy), plus plugins for the{trigger, single-line constraint, ValueSections bridge, history, and
chip-click → state modal. Adds
lexical+@lexical/reactto@beak/ui. One backwards-compatible addition toVariableSelector(anoptional
anchorRectprop) so V2 can position the popover by caretrect rather than the legacy
partIndexDOM lookup. Legacy code isuntouched otherwise.
VariableSelector'sposition effect now refires on
partIndex/offsetchanges so thepopover tracks the caret as the user types
{query};VariableChipNodeemits
<span>rather than<div>to remain valid inside Lexical's<p>;SingleLinePlugin's doc comment now describes the real Cmd+Enterflow (host-handled, not global);
feature-flagdispatches its changeevent unconditionally so private-mode/quota failures don't silence the
toggle.
What's intentionally NOT in this PR
Playwright suites. See ADR-0002 §5 for the ordered checklist.
COPY_COMMAND/PASTE_COMMANDplugin for V2.PlainTextPlugin'spaste handler reads only
text/plain, so the chip-aware clipboardround-trip (spec scenarios
variable-input.feature:164and:176)currently fails — chips are dropped on paste. Known gap; lands with
the cutover.
LineBreakNodetransform. Multi-line paste keeps the datanewline-stripped (via the
TextNodetransform inSingleLinePlugin)but leaves invisible
<br>remnants in the editor tree. CSS hidesthem; lands with the cutover.
Test plan
Playground (Cmd+K → "Variable input lab"):
{+ query + Enter inserts a chip and removes the trigger text.chip's
data-payloadreflects the new value.text-security.places the caret correctly.
Linked