fix(apollo-react): restore inline preset rename/delete actions in NodePropertyTrigger#810
Merged
CalinaCristian merged 2 commits intoJun 12, 2026
Conversation
…ePropertyTrigger The review-feedback pass on #781 replaced the inline pencil/trash buttons on preset rows with indented per-preset 'Rename {label}' / 'Delete {label}' menu items. Product/design prefers the inline affordance — restore it: - preset rows render pencil/trash icon buttons inline (pointer events stopped so the Radix item's pointerup-select doesn't apply the preset) - truncated preset labels get a native title tooltip - catalog: rename_preset / delete_preset ids replace the *_with_label pair - story: wire onPresetRename (the pencil only renders when provided) with a bounded '(n)' counter demo handler Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
Pull request overview
Restores inline rename/delete affordances (pencil/trash) for preset rows in the Canvas NodePropertyTrigger dropdown, aligning the interaction with the design prototype and reverting the prior indented menu-item approach.
Changes:
- Render inline pencil/trash icon buttons within each preset row and stop pointer events so clicking them doesn’t trigger preset apply/close.
- Add native
titletooltip for truncated preset labels. - Update i18n message IDs for rename/delete preset actions and wire
onPresetRenamein Storybook.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/locales/en.json | Replaces rename/delete preset message IDs used by the control. |
| packages/apollo-react/src/canvas/controls/NodePropertyTrigger/NodePropertyTrigger.tsx | Reintroduces inline rename/delete icon buttons on preset rows + truncated-label title tooltip. |
| packages/apollo-react/src/canvas/controls/NodePropertyTrigger/NodePropertyTrigger.test.tsx | Updates tests to target the new inline buttons and keep-menu-open behavior. |
| packages/apollo-react/src/canvas/controls/NodePropertyTrigger/NodePropertyTrigger.stories.tsx | Adds a demo onPresetRename handler so the pencil button is visible/interactive in Storybook. |
Copilot review: multiple presets render multiple icon buttons, so generic
'Rename preset' / 'Delete preset' accessible names are ambiguous for AT and
brittle for queries. Use the *_with_label catalog ids ('Rename {label}' /
'Delete {label}') for title/aria-label on the inline buttons.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CalinaCristian
deleted the
fix/node-property-trigger-inline-preset-actions
branch
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.
What
Restores the inline pencil/trash action buttons on
NodePropertyTriggerpreset rows. The review-feedback pass on #781 replaced them with indented per-presetRename {label}/Delete {label}menu items — product/design prefers the inline affordance (matches the flow design prototype).Changes
pointerdown/pointerup/click) so the Radix item's pointerup-select doesn't apply the preset or close the menu.titletooltip (same pattern as the icon buttons' hints).canvas.property_trigger.rename_preset/delete_presetids replace the*_with_labelpair.onPresetRename(the pencil only renders when the handler is provided, so Storybook never showed it) with a bounded(n)counter demo handler.Notes for review
The indented-items variant was arguably the stricter ARIA shape (no nested interactive elements inside a
menuitem). The inline buttons keep their ownaria-labels and remain keyboard-reachable, and this is the interaction the design spec shows — flagging the tradeoff explicitly rather than silently re-litigating the review.Test plan
vitest run src/canvas/controls/NodePropertyTrigger— 27 tests pass (rename/delete cases reverted to the inline-button contract).(n)counter; delete removes the row; menu stays open for both.🤖 Generated with Claude Code