feat: UX for double click on nodes [AB-879] #1245
Conversation
WalkthroughAdds per-component editor expansion state and UI controls, auto-focuses the embedded editor in half-screen on mount, enables double-click node navigation/expansion with tracking/toasts, surfaces blueprint keys in the UI, and generates incremental blueprint keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node as BlueprintsNode
participant Manager as BuilderManager
participant Root as BlueprintsRoot
participant Toast as ToastService
User->>Node: double-click node
alt code/state/return/log node
Node->>Manager: ssbm.expandedEditorForComponent = componentId
Manager-->>Node: reactive update
Node->>Node: expand and focus editor (WdsFieldWrapper handles expansion)
Node->>Manager: tracking("dbl_click_for_code_editor_opened" / "dbl_click_for_value_opened")
else runblueprint node
Node->>Manager: goToComponentParentPage(componentKey)
Manager->>Root: activate blueprint page (after nextTick)
Root-->>Manager: page activated
Node->>Toast: push success toast("Navigated to blueprint root")
Node->>Manager: tracking("dbl_click_for_blueprint_navigated")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/ui/src/builder/BuilderEmbeddedCodeEditor.vue (1)
118-130: LGTM with a minor optimization suggestion.The logic correctly focuses the editor and positions the cursor at the end of content for half-screen modals. Consider caching
editor.getModel().getLineCount()to avoid calling it three times.// when in modal, focus the editor and set the cursor to the last line if (props.variant === "half-screen") { editor.focus(); + const lineCount = editor.getModel().getLineCount(); editor.setPosition({ - lineNumber: editor.getModel().getLineCount(), - column: editor - .getModel() - .getLineLastNonWhitespaceColumn( - editor.getModel().getLineCount(), - ), + lineNumber: lineCount, + column: editor.getModel().getLineLastNonWhitespaceColumn(lineCount), }); }src/ui/src/builder/builderManager.ts (1)
421-428: Preferfind()overfilter()[0]for checking existence.Using
filter(...)?.[0]to check for existence is less idiomatic thanfind(). Since you're only checking if any matching entry exists,some()would be even cleaner.const isActive = Boolean( - logEntries.filter((entry) => { - return ( - entry?.blueprintExecution?.runId === runId && - !entry?.blueprintExecution?.exit - ); - })?.[0], + logEntries.some( + (entry) => + entry?.blueprintExecution?.runId === runId && + !entry?.blueprintExecution?.exit, + ), );src/ui/src/components/blueprints/BlueprintsRoot.vue (1)
3-15: LGTM with a minor simplification note.The title container correctly displays the blueprint key with fallback to id. The condition
displayedBlueprintId || displayedBlueprintKeycould be simplified to justdisplayedBlueprintIdsincedisplayedBlueprintKeyis derived fromdisplayedBlueprintIdand will be undefined when the parent is null.<div - v-if="displayedBlueprintId || displayedBlueprintKey" + v-if="displayedBlueprintId" class="BlueprintsRoot_titleContainer" >src/ui/src/components/blueprints/abstract/BlueprintsNode.vue (1)
417-448: Good implementation with one edge case to consider.The double-click handler correctly distinguishes between component types and implements the appropriate behavior. The guards for empty
bpKeyand missing blueprint are well placed.However, on line 445,
blueprint.content.keycould theoretically be undefined if a blueprint exists but has no key set (edge case with legacy data). Consider using the same fallback pattern as inBlueprintsRoot.vue:pushToast({ type: "success", - message: `Navigated to ${blueprint.content.key} blueprint`, + message: `Navigated to ${blueprint.content?.key ?? blueprint.id} blueprint`, });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/ui/src/builder/BuilderEmbeddedCodeEditor.vue(1 hunks)src/ui/src/builder/builderManager.ts(2 hunks)src/ui/src/builder/settings/BuilderSettingsProperties.vue(3 hunks)src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue(1 hunks)src/ui/src/components/blueprints/BlueprintsRoot.vue(3 hunks)src/ui/src/components/blueprints/abstract/BlueprintsNode.vue(4 hunks)src/ui/src/wds/WdsFieldWrapper.vue(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-05T12:07:52.320Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1059
File: src/ui/src/builder/settings/BuilderFieldsAlign.vue:27-27
Timestamp: 2025-08-05T12:07:52.320Z
Learning: In Vue 3 templates, refs are automatically unwrapped, so when binding a ref to a template property, you should use the ref directly (e.g., `:value="fieldViewModel"`) instead of explicitly accessing the .value property (e.g., `:value="fieldViewModel.value"`). The .value access is only needed in script contexts.
Applied to files:
src/ui/src/wds/WdsFieldWrapper.vuesrc/ui/src/components/blueprints/abstract/BlueprintsNode.vue
📚 Learning: 2025-08-05T12:07:52.320Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1059
File: src/ui/src/builder/settings/BuilderFieldsAlign.vue:27-27
Timestamp: 2025-08-05T12:07:52.320Z
Learning: In Vue 3 templates, refs are automatically unwrapped, so when binding a ref to a template property, you can use the ref directly (e.g., `:value="fieldViewModel"`) instead of explicitly accessing the .value property (e.g., `:value="fieldViewModel.value"`).
Applied to files:
src/ui/src/wds/WdsFieldWrapper.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
🔇 Additional comments (10)
src/ui/src/builder/settings/BuilderSettingsProperties.vue (3)
292-297: LGTM!The computed property correctly determines whether the field editor should be expanded based on the current component selection.
93-93: LGTM!The
should-expandbinding correctly combines the expansibility check with the per-component expansion state.
402-410: LGTM!The handlers correctly manage both the global per-component expansion state and the local field tracking set.
src/ui/src/builder/builderManager.ts (1)
438-438: LGTM!The new
expandedEditorForComponentstate is correctly typed and positioned in the builder object, enabling per-component editor expansion tracking across the UI.src/ui/src/wds/WdsFieldWrapper.vue (1)
3-6: LGTM!The modal structure with the slot correctly placed inside looks good.
src/ui/src/components/blueprints/BlueprintsRoot.vue (2)
64-76: LGTM!The computed property correctly extracts the key from the displayed blueprint, and the
onMountedhook properly synchronizes the active page state.
89-115: LGTM!The CSS correctly positions the title as a non-interactive floating element with appropriate styling.
src/ui/src/components/blueprints/abstract/BlueprintsNode.vue (2)
14-14: LGTM!Double-click handler correctly wired to the root element.
155-161: LGTM!The new imports and destructured hooks are correctly set up for the navigation and toast functionality.
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (1)
172-176: TheappendSelectioncall appends to the current selection instead of replacing it, differing from theaddPagefunction.
addPageusessetSelection(pageId)which replaces the current selection, whileaddBlueprintusesappendSelection(pageId)which adds the new blueprint to the existing selection. Without documentation explaining this difference, verify whether appending to the selection for newly created blueprints is the intended behavior.
HackerOne Code Security Review🟢 Scan Complete: 5 Issue(s) Here's how the code changes were interpreted and info about the tools used for scanning. 📖 Summary of ChangesThe changes primarily focus on enhancing the builder and blueprint components' user interaction and state management. Key modifications include adding expanded editor tracking, implementing unique blueprint key generation, improving component selection methods, and introducing new event handlers for navigation and expansion. The updates aim to provide more flexible and interactive editing experiences across various UI components.
ℹ️ Issues DetectedNOTE: These may not require action! Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem. How will I know if something is a problem?
🧰 Analysis tools
⏱️ Latest scan covered changes up to commit c287a60 (latest) |
|
✅ Graham C reviewed all the included code changes and associated automation findings and determined that there were no immediately actionable security flaws. Note that they will continue to be notified of any new commits or comments and follow up as needed throughout the duration of this pull request's lifecycle.
Reviewed with ❤️ by PullRequest |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (1)
178-181: Key collision risk remains unaddressed.This issue was already flagged in a previous review. The current implementation using
allBlueprints.value.lengthwill generate duplicate keys when blueprints are deleted. Please refer to the existing review comment on these lines for the recommended fix.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
- GitHub Check: tests (firefox)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/ui/src/builder/BuilderApp.vue(1 hunks)src/ui/src/components/blueprints/BlueprintsRoot.vue(4 hunks)src/ui/src/components/blueprints/abstract/BlueprintsNode.vue(6 hunks)src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue(4 hunks)src/ui/src/composables/useWriterTracking.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-05T12:07:52.320Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1059
File: src/ui/src/builder/settings/BuilderFieldsAlign.vue:27-27
Timestamp: 2025-08-05T12:07:52.320Z
Learning: In Vue 3 templates, refs are automatically unwrapped, so when binding a ref to a template property, you should use the ref directly (e.g., `:value="fieldViewModel"`) instead of explicitly accessing the .value property (e.g., `:value="fieldViewModel.value"`). The .value access is only needed in script contexts.
Applied to files:
src/ui/src/components/blueprints/abstract/BlueprintsNode.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.13)
- GitHub Check: tests (firefox)
- GitHub Check: build (3.11)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: tests (chromium)
🔇 Additional comments (9)
src/ui/src/builder/BuilderApp.vue (1)
271-274: LGTM! Proper state cleanup on Escape.Clearing
expandedEditorForComponentalongside selection provides consistent UX when the user presses Escape to dismiss UI elements.src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue (2)
43-48: LGTM! Proper async flow for editor expansion.The implementation correctly:
- Selects the component first
- Awaits DOM update with
nextTick()- Expands the editor
- Tracks the user action
107-117: LGTM! Button properly gated and configured.The editor-opening button is correctly:
- Conditionally rendered based on
showOpenEditorOption- Configured with appropriate tooltip and icon
- Marked as unselectable to prevent interference with node selection
src/ui/src/composables/useWriterTracking.ts (1)
44-48: LGTM! Event names follow conventions.The new tracking events are well-named and align with the double-click interactions being implemented across blueprint nodes and editors.
src/ui/src/components/blueprints/BlueprintsRoot.vue (3)
68-71: LGTM! Safe optional chaining.The computed property safely accesses nested properties with optional chaining, preventing runtime errors when the blueprint is not found.
96-122: LGTM! Well-styled non-interactive overlay.The title container styling is appropriate:
- Absolute positioning keeps it out of document flow
pointer-events: noneanduser-select: noneprevent interaction- Clear visual hierarchy with prefix styling
- Proper z-index for layering
73-83: No action needed —useComponentInformationalready handles undefinedfirstSelectedIdsafely.The function properly handles null or undefined
componentIdusingunref()to unwrap the parameter, followed by an explicitif (!id) return;guard that returns undefined when no component is selected. The calling code inBlueprintsRoot.vuealso includes a defensive check at line 78:if (!component.value).src/ui/src/components/blueprints/abstract/BlueprintsNode.vue (2)
14-14: LGTM! Double-click handler attached.The double-click event handler is properly bound to implement the navigation and editor-opening features.
424-461: Add error handling and validation for blueprint navigation edge cases.The code has three potential issues:
Empty string truthy check: The check
!bpKeydoes not catch ifblueprintKeyis somehow defined as an empty string through data binding issues.Duplicate blueprint keys: No verification that blueprint keys are globally unique. Multiple blueprints with the same key could cause unpredictable navigation behavior.
Unhandled navigation failure: If
goToComponentParentPagefails or throws an error, the function will crash without user feedback.Consider:
- Validating
blueprintKeyis a non-empty string before attempting navigation- Enforcing blueprint key uniqueness at creation time or adding a check to warn of duplicates
- Wrapping
goToComponentParentPagein try-catch and providing error feedback
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (1)
157-177: Operation order inconsistency remains unresolved.The previous review flagged that
addBlueprinthas a different operation order thanaddPageandaddSharedBlueprint:
addPage(lines 149-155):setActivePageId→nextTick→setSelectionaddSharedBlueprint(lines 193-203):setActivePageId→nextTick→setSelectionaddBlueprint(lines 157-177):nextTick→setActivePageId→setSelectionSince
createAndInsertComponentis synchronous, callingnextTick()beforesetActivePageId()appears unnecessary. Align with the pattern used byaddPageandaddSharedBlueprintfor consistency.🔎 Proposed fix to align operation order
async function addBlueprint() { const key = getNewBlueprintKey(); const pageId = createAndInsertComponent( "blueprints_blueprint", "blueprints_root", undefined, key ? { content: { key, }, } : undefined, ); - await nextTick(); wf.setActivePageId(pageId); + await nextTick(); wfbm.setSelection(pageId); tracking.track("blueprints_new_added"); }
🧹 Nitpick comments (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue (1)
179-191: Simplify the fallback logic for blueprint key generation.The implementation correctly addresses the key collision risk from the previous review. However, the fallback value
allBlueprints.value.length + 10(line 189) introduces unnecessary gaps in the numbering sequence.If no existing blueprints have numeric
BLUEPRINT_keys, usingallBlueprints.value.length + 10could create confusing numbering. For example, if 5 blueprints exist without numeric keys, the next blueprint will beBLUEPRINT_16instead ofBLUEPRINT_1.Consider using
0as the fallback for cleaner, sequential numbering.🔎 Proposed simplification
function getNewBlueprintKey() { const existingKeys = allBlueprints.value .map((bp) => bp.content?.key) .filter((key) => key?.startsWith("BLUEPRINT_")) .map((key) => Number.parseInt(key.replace("BLUEPRINT_", ""), 10)) .filter((num) => !Number.isNaN(num)); const maxIndex = existingKeys.length > 0 ? Math.max(...existingKeys) - : allBlueprints.value.length + 10; + : 0; return `BLUEPRINT_${maxIndex + 1}`; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ui/src/builder/sidebar/BuilderSidebarComponentTree.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.13)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
| const expandedFields = ref(new Set()); | ||
| // Check if the code field should be expanded for the current component |
There was a problem hiding this comment.
let's remove code self explained
| const bpKey = component.value.content?.blueprintKey ?? ""; | ||
| if (!bpKey) return; | ||
| // Find the blueprint component by its key |
There was a problem hiding this comment.
code self explained let's remove comment
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ui/src/components/blueprints/abstract/BlueprintsNode.vue (1)
182-184:isCodeComponentdefinition is too narrow (duplicate concern).This issue was raised in a previous review but remains unaddressed. The computed property only checks for
blueprints_code, but the double-click handler (lines 426-430) supports four component types:blueprints_code,blueprints_setstate,blueprints_returnvalue, andblueprints_logmessage.As a result, the "Open editor" button (line 114) won't appear for the latter three types, even though they support editor expansion on double-click.
🔎 Apply this diff to support all four component types
const isCodeComponent = computed(() => { - return component.value?.type === "blueprints_code"; + return ( + component.value?.type === "blueprints_code" || + component.value?.type === "blueprints_setstate" || + component.value?.type === "blueprints_returnvalue" || + component.value?.type === "blueprints_logmessage" + ); });
🧹 Nitpick comments (1)
src/ui/src/builder/settings/BuilderSettingsProperties.vue (1)
406-409: Verify: Should component-level expansion state clear when ANY field shrinks?The current implementation clears
ssbm.expandedEditorForComponent.valuewhen any field callshandleShrink, even if other fields might still be expanded (as tracked in theexpandedFieldsSet).Should the component-level expansion state persist until ALL fields are shrunk, or is it correct to clear it immediately when any single field shrinks?
Consider whether the logic should be:
function handleShrink(fieldKey: string) { expandedFields.value.delete(fieldKey); // Only clear component-level state if no fields remain expanded if (expandedFields.value.size === 0) { ssbm.expandedEditorForComponent.value = null; } }This depends on the intended UX behavior for per-component expansion tracking.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/ui/src/builder/settings/BuilderSettingsProperties.vue(3 hunks)src/ui/src/components/blueprints/abstract/BlueprintsNode.vue(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-05T12:07:52.320Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1059
File: src/ui/src/builder/settings/BuilderFieldsAlign.vue:27-27
Timestamp: 2025-08-05T12:07:52.320Z
Learning: In Vue 3 templates, refs are automatically unwrapped, so when binding a ref to a template property, you should use the ref directly (e.g., `:value="fieldViewModel"`) instead of explicitly accessing the .value property (e.g., `:value="fieldViewModel.value"`). The .value access is only needed in script contexts.
Applied to files:
src/ui/src/components/blueprints/abstract/BlueprintsNode.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: tests (firefox)
- GitHub Check: tests (chromium)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.11)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
🔇 Additional comments (4)
src/ui/src/components/blueprints/abstract/BlueprintsNode.vue (1)
424-460: Well-structured double-click implementation.The implementation correctly handles two distinct behaviors:
- For code/setstate/returnvalue/logmessage components: expands the editor when the component is selected.
- For runblueprint components: navigates to the target blueprint with appropriate feedback (toast + tracking).
The early returns (lines 443, 449) prevent errors when data is missing, and the use of
nextTick(line 451) ensures the DOM is ready before selecting the blueprint in the tree.src/ui/src/builder/settings/BuilderSettingsProperties.vue (3)
292-296: LGTM! Clean per-component expansion state.The computed property correctly tracks whether the current component has an expanded editor by comparing against the builder manager's reactive state.
401-404: LGTM! Proper null handling in expansion logic.The use of optional chaining and nullish coalescing ensures that
expandedEditorForComponentis always either a string or null, preventing undefined values. If no component is selected, setting it to null correctly prevents expansion.
93-93: All expansible fields receiveshouldExpand=truewhen any field in the component expands.When
handleExpand(fieldKey)is called for a single field, it setsssbm.expandedEditorForComponent.valueto the component ID, makingisFieldExpandedtrue for the entire component. This causes ALL expansible fields to receiveshouldExpand=truesimultaneously.The
expandedFieldsSet tracks which specific field was clicked, but the:should-expandbinding ignores this, using only the component-levelisFieldExpandedflag instead. This means multiple field modals could open simultaneously rather than just the clicked field.Consider using the
expandedFieldsSet to controlshouldExpandon a per-field basis, or clarify whether expanding all expansible fields simultaneously is the intended design.
| const maxIndex = | ||
| existingKeys.length > 0 | ||
| ? Math.max(...existingKeys) | ||
| : allBlueprints.value.length + 10; |
There was a problem hiding this comment.
Why +10 ?
| : allBlueprints.value.length + 10; | |
| : allBlueprints.value.length; |
There was a problem hiding this comment.
Not the best solution but in that fallback scenario, it will give some gap to prevent same name matching.
| | "dbl_click_for_code_editor_opened" | ||
| | "dbl_click_for_value_opened" | ||
| | "button_click_for_code_editor_opened" | ||
| | "dbl_click_for_blueprint_navigated"; |
There was a problem hiding this comment.
We need to stick to the current naming convention
| | "dbl_click_for_code_editor_opened" | |
| | "dbl_click_for_value_opened" | |
| | "button_click_for_code_editor_opened" | |
| | "dbl_click_for_blueprint_navigated"; | |
| | "blueprints_dbl_click_for_code_editor_opened" | |
| | "blueprints_dbl_click_for_value_opened" | |
| | "blueprints_button_click_for_code_editor_opened" | |
| | "blueprints_dbl_click_for_blueprint_navigated"; |
| () => props.shouldExpand, | ||
| (newVal) => { | ||
| if (newVal && !isExpanded.value) { | ||
| handleExpansion(); | ||
| } |
There was a problem hiding this comment.
| () => props.shouldExpand, | |
| (newVal) => { | |
| if (newVal && !isExpanded.value) { | |
| handleExpansion(); | |
| } | |
| () => props.shouldExpand && !isExpanded.value, | |
| (newVal) => { | |
| if (newVal) handleExpansion(); |
| <script setup lang="ts"> | ||
| import { computed, inject, onMounted } from "vue"; | ||
| import injectionKeys from "@/injectionKeys"; | ||
| import { getSourceBlueprintName } from "@/builder/useComponentDescription"; |
There was a problem hiding this comment.
| import { getSourceBlueprintName } from "@/builder/useComponentDescription"; |
| </script> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { computed, inject, onMounted } from "vue"; |
There was a problem hiding this comment.
| import { computed, inject, onMounted } from "vue"; | |
| import { computed, inject } from "vue"; |
src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue
Outdated
Show resolved
Hide resolved
Co-authored-by: Alexandre Rousseau <alexandre@rsseau.fr>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue (2)
43-48: Consider adding error handling for better user feedback.The async function performs multiple operations without error handling. While these internal operations are unlikely to fail, wrapping them in a try-catch block would provide better user feedback if something goes wrong.
🔎 Suggested error handling
async function openEditorForComponent() { + try { wfbm.setSelection(componentId, undefined, "click"); await nextTick(); wfbm.expandedEditorForComponent.value = componentId; tracking.track("button_click_for_code_editor_opened"); + } catch (error) { + console.error("Failed to open editor for component:", error); + // Optionally: show a toast notification to the user + } }
107-117: Consider using consistent attribute binding style.The new button uses
data-writer-unselectable(line 112) while other buttons in the same file use:data-writer-unselectable="true"(lines 123, 134). While both approaches work for boolean attributes, consistency within the file would improve maintainability.🔎 Suggested consistency fix
<WdsButton v-if="props.showOpenEditorOption" size="smallIcon" variant="neutral" data-writer-tooltip-placement="bottom" - data-writer-unselectable + :data-writer-unselectable="true" data-writer-tooltip="Open editor" @click.prevent="openEditorForComponent" > <WdsIcon name="code" /> </WdsButton>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: tests (webkit)
- GitHub Check: tests (chromium)
- GitHub Check: tests (firefox)
🔇 Additional comments (2)
src/ui/src/components/blueprints/abstract/BlueprintsNodeActions.vue (2)
2-2: LGTM: Import addition is correct.The
nextTickimport is necessary for the newopenEditorForComponentfunction and follows Vue best practices.
25-25: LGTM: Prop definition follows existing patterns.The new prop is consistent with the existing
showDisplayErrorOptionprop and properly typed.
Summary by CodeRabbit
New Features
Bug Fixes / UX
Telemetry
✏️ Tip: You can customize this high-level summary in your review settings.