feat: hover over blueprint to delete - AB-939#1270
Conversation
📝 WalkthroughWalkthroughAdds a right-click context dropdown to the component tree branch component exposing a conditional "Delete" action for blueprint-type components, wires dropdown-select handling, gates visibility by delete permission, and emits a new "delete" event; also contains a trivial .gitignore line change. Changes
Sequence Diagram(s)(Skipped — change is confined to a single UI component and does not introduce a multi-component sequential flow warranting a diagram.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/ui/src/builder/sidebar/BuilderSidebarComponentTreeBranch.vue`:
- Around line 20-26: The binding
:right-click-options="rightClickDropdownOptions" in
BuilderSidebarComponentTreeBranch.vue is unused because BuilderTree.vue doesn't
declare that prop; either remove that binding from
BuilderSidebarComponentTreeBranch.vue, or add a prop (e.g., rightClickOptions)
to BuilderTree.vue and wire it into its right-click handling code (handle
context menu / emit events) so rightClickDropdownOptions is respected; if you
add the prop, gate its value with isDeleteAllowed(props.componentId) so the
Delete option is only surfaced when allowed.
🧹 Nitpick comments (2)
src/ui/src/builder/BuilderTree.vue (1)
10-11: Make the dropdown keyboard-accessible (focus in/out).Right now the three‑dots menu only appears on hover, so keyboard users can’t discover or trigger delete from the tree. Consider toggling
isMainHoveredon focus as well.♿ Proposed tweak
- `@mouseenter`="handleMouseEnter" - `@mouseleave`="handleMouseLeave" + `@mouseenter`="handleMouseEnter" + `@mouseleave`="handleMouseLeave" + `@focusin`="handleFocusIn" + `@focusout`="handleFocusOut"function handleMouseLeave(ev: MouseEvent) { isMainHovered.value = false; emit("mouseleave", ev); } + +function handleFocusIn(_ev: FocusEvent) { + isMainHovered.value = true; +} + +function handleFocusOut(_ev: FocusEvent) { + isMainHovered.value = false; +}Also applies to: 142-149
src/ui/src/builder/sidebar/BuilderSidebarComponentTreeBranch.vue (1)
225-227: Guard delete by dropdown key.
handleDropdownSelectcurrently deletes regardless of which option was picked. If options expand in the future, this will delete unintentionally.✅ Safer handler
-function handleDropdownSelect() { - handleDelete(); -} +function handleDropdownSelect(key: string) { + if (key !== "delete") return; + handleDelete(); +}
| @dragstart="handleDragStart" | ||
| @dragend="handleDragEnd" | ||
| @drop="handleDrop" | ||
| @delete="handleDelete" |
There was a problem hiding this comment.
BuilderTree has a delete event, but it's not used. I think you can remove this line and update BuilderTree
src/ui/src/builder/BuilderTree.vue
Outdated
| dropdownSelect: (key: string) => typeof key === "string", | ||
| mouseenter: (ev: MouseEvent) => !!ev, | ||
| mouseleave: (ev: MouseEvent) => !!ev, | ||
| delete: () => true, |
There was a problem hiding this comment.
These changes are not needed; the feature would work without them. I suggest you revert these changes since it's not necessary.
git checkout origin/dev -- src/ui/src/builder/BuilderTree.vueThere was a problem hiding this comment.
Yup I see, will do
| function handleDropdownSelect() { | ||
| handleDelete(); |
There was a problem hiding this comment.
| function handleDropdownSelect() { | |
| handleDelete(); | |
| function handleDropdownSelect(action: string) { | |
| if (action === "delete") handleDelete(); |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/ui/src/builder/sidebar/BuilderSidebarComponentTreeBranch.vue`:
- Line 28: The template is emitting
`@dropdown-select`="handleDropdownSelect($event, key)" but `key` is undefined;
update the template to pass the correct component identifier (e.g., use
`props.componentId` or the local `component.id`) so handleDropdownSelect
receives the actual id. Locate the binding in
BuilderSidebarComponentTreeBranch.vue and replace the second argument with the
proper identifier used elsewhere in this component (referenced by the handler
name handleDropdownSelect) so the delete emit sends the real component id
instead of undefined.
- Line 121: The emits constant from defineEmits is misnamed relative to usage in
expand(): change the emission usage so they match; either rename the defined
constant from emits to emit (i.e., use const emit = defineEmits(["expandBranch",
"delete"]) ) or update the expand() function to call emits("expandBranch", ...)
instead of emit(...); locate the defineEmits call and the expand() function in
BuilderSidebarComponentTreeBranch.vue and make one of these two consistent fixes
so the emitted event calls reference the same identifier.
♻️ Duplicate comments (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTreeBranch.vue (1)
20-20: Remove unusedright-click-optionsprop.This binding was already identified as unused in a previous review and confirmed to be removable.
BuilderTree.vuedoes not define this prop.
src/ui/src/builder/sidebar/BuilderSidebarComponentTreeBranch.vue
Outdated
Show resolved
Hide resolved
This reverts commit ad748d0.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.