fix: textarea stays disabled after link disconnect on promoted widgets#9199
fix: textarea stays disabled after link disconnect on promoted widgets#9199christian-byrne merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 549 passed, 0 failed · 4 flaky📊 Browser Reports
|
📦 Bundle: 4.41 MB gzip 🔴 +38 BDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 971 kB (baseline 970 kB) • 🔴 +112 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.54 MB (baseline 2.54 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.7 MB (baseline 7.7 MB) • ⚪ 0 BBundles that do not match a named category
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/composables/graph/useGraphNodeManager.ts (1)
385-388: Clear staleslotMetadatawhen a slot lookup misses.At Line 387, metadata is only updated on matches. If a slot is removed/renamed, old
slotMetadatacan persist and leave linked-dependent UI state stale.♻️ Suggested defensive update
for (const widget of currentData.widgets ?? []) { const slotInfo = slotMetadata.get(widget.slotName ?? widget.name) - if (slotInfo) widget.slotMetadata = slotInfo + if (slotInfo) { + widget.slotMetadata = slotInfo + } else { + delete widget.slotMetadata + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/graph/useGraphNodeManager.ts` around lines 385 - 388, The loop that sets widget.slotMetadata only assigns when slotInfo is found, leaving stale metadata when a slot lookup misses; in the loop over currentData.widgets (where slotInfo = slotMetadata.get(widget.slotName ?? widget.name)) explicitly clear or remove widget.slotMetadata (e.g., set to undefined or delete the property) when slotInfo is falsy so renamed/removed slots don't retain old metadata; update the block in useGraphNodeManager.ts that iterates currentData.widgets to handle both match and miss cases for widget.slotMetadata.src/composables/graph/useGraphNodeManager.test.ts (1)
184-229: Prefer real promotion setup over manual promoted-field injection.At Line 199 and Line 200, mutating
BaseWidgetvia unknown-cast works, but using the actual promotion path would reduce drift risk between test doubles and runtime promoted widget shape/type-guard behavior.As per coding guidelines "Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/graph/useGraphNodeManager.test.ts` around lines 184 - 229, The test manually mutates the widget by casting to unknown and setting sourceNodeId/sourceWidgetName on the BaseWidget (the `widget` returned by `node.addWidget`) which diverges from runtime promoted-widget creation; instead, use the real promotion API/path so `isPromotedWidgetView` and `safeWidgetMapper` see the exact runtime shape (e.g., create the promoted input via the project’s promotion helper such as promoteInput/promoteWidget or the node/graph method that performs promotion rather than setting `sourceNodeId`/`sourceWidgetName` directly) so the test exercises actual promotion behavior and removes the unknown-cast mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/composables/graph/useGraphNodeManager.test.ts`:
- Around line 184-229: The test manually mutates the widget by casting to
unknown and setting sourceNodeId/sourceWidgetName on the BaseWidget (the
`widget` returned by `node.addWidget`) which diverges from runtime
promoted-widget creation; instead, use the real promotion API/path so
`isPromotedWidgetView` and `safeWidgetMapper` see the exact runtime shape (e.g.,
create the promoted input via the project’s promotion helper such as
promoteInput/promoteWidget or the node/graph method that performs promotion
rather than setting `sourceNodeId`/`sourceWidgetName` directly) so the test
exercises actual promotion behavior and removes the unknown-cast mutations.
In `@src/composables/graph/useGraphNodeManager.ts`:
- Around line 385-388: The loop that sets widget.slotMetadata only assigns when
slotInfo is found, leaving stale metadata when a slot lookup misses; in the loop
over currentData.widgets (where slotInfo = slotMetadata.get(widget.slotName ??
widget.name)) explicitly clear or remove widget.slotMetadata (e.g., set to
undefined or delete the property) when slotInfo is falsy so renamed/removed
slots don't retain old metadata; update the block in useGraphNodeManager.ts that
iterates currentData.widgets to handle both match and miss cases for
widget.slotMetadata.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/composables/graph/useGraphNodeManager.test.tssrc/composables/graph/useGraphNodeManager.ts
refreshNodeSlots used SafeWidgetData.name for slot metadata lookups. For promoted widgets, name is sourceWidgetName (interior name) which differs from the subgraph node's input slot widget name. Add slotName field to SafeWidgetData to track the original LiteGraph widget name, and use it in refreshNodeSlots for correct matching. Amp-Thread-ID: https://ampcode.com/threads/T-019c93e3-1cef-72cc-b7cc-a2f067070ff6
06ad7e7 to
f81f6ed
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/composables/graph/useGraphNodeManager.test.ts (1)
155-159: Simplify redundant boolean ternary.
linked ? true : falseis equivalent to!!linked.♻️ Proposed simplification
- return widget?.slotMetadata?.linked ? true : false + return !!widget?.slotMetadata?.linked🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/graph/useGraphNodeManager.test.ts` around lines 155 - 159, In derivedDisabled, simplify the redundant ternary by converting the nullable linked property to a boolean directly (e.g., use !!widget?.slotMetadata?.linked or Boolean(widget?.slotMetadata?.linked)) instead of "widget?.slotMetadata?.linked ? true : false"; update the computed that references nodeData, widgets, widget and slotMetadata to use the boolean cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/composables/graph/useGraphNodeManager.test.ts`:
- Around line 155-159: In derivedDisabled, simplify the redundant ternary by
converting the nullable linked property to a boolean directly (e.g., use
!!widget?.slotMetadata?.linked or Boolean(widget?.slotMetadata?.linked)) instead
of "widget?.slotMetadata?.linked ? true : false"; update the computed that
references nodeData, widgets, widget and slotMetadata to use the boolean cast.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/composables/graph/useGraphNodeManager.test.tssrc/composables/graph/useGraphNodeManager.ts
#9199) ## Summary Fix textarea widgets staying disabled after disconnecting a link on promoted widgets in subgraphs. ## Changes - **What**: `refreshNodeSlots` used `SafeWidgetData.name` for slot metadata lookups, but for promoted widgets this is `sourceWidgetName` (the interior widget name), which doesn't match the subgraph node's input slot widget name. Added `slotName` field to `SafeWidgetData` to track the original LiteGraph widget name, and updated `refreshNodeSlots` to use `slotName ?? name` for correct matching. ## Review Focus The key change is the `slotName` field on `SafeWidgetData` — it's only populated when `name !== widget.name` (i.e., for promoted widgets). The `refreshNodeSlots` function now uses `widget.slotName ?? widget.name` to look up slot metadata, ensuring promoted widgets correctly update their `linked` state on disconnect. Fixes #8818 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9199-fix-textarea-stays-disabled-after-link-disconnect-on-promoted-widgets-3126d73d3650813db499c227e6587aca) by [Unito](https://www.unito.io)
Summary
Fix textarea widgets staying disabled after disconnecting a link on promoted widgets in subgraphs.
Changes
refreshNodeSlotsusedSafeWidgetData.namefor slot metadata lookups, but for promoted widgets this issourceWidgetName(the interior widget name), which doesn't match the subgraph node's input slot widget name. AddedslotNamefield toSafeWidgetDatato track the original LiteGraph widget name, and updatedrefreshNodeSlotsto useslotName ?? namefor correct matching.Review Focus
The key change is the
slotNamefield onSafeWidgetData— it's only populated whenname !== widget.name(i.e., for promoted widgets). TherefreshNodeSlotsfunction now useswidget.slotName ?? widget.nameto look up slot metadata, ensuring promoted widgets correctly update theirlinkedstate on disconnect.Fixes #8818
┆Issue is synchronized with this Notion page by Unito