Skip to content

Conversation

@ivicac
Copy link
Contributor

@ivicac ivicac commented Nov 11, 2025

No description provided.

Copilot finished reviewing on behalf of ivicac November 11, 2025 21:18
@sonarqubecloud
Copy link

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the WorkflowNodeDetailsPanel to remain open when the cluster element editor is opened, improving the workflow editing experience.

  • Removed early return in useNodeClick that prevented the details panel from opening when clicking a cluster root node
  • Added optional closeButton prop to WorkflowNodeDetailsPanel to allow custom close button behavior
  • Modified WorkflowEditorLayout to always render WorkflowNodeDetailsPanel within the cluster elements canvas dialog

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
client/src/pages/platform/workflow-editor/hooks/useNodeClick.ts Removed logic that closed the details panel and cleared the current component when opening cluster editor
client/src/pages/platform/workflow-editor/components/WorkflowNodeDetailsPanel.tsx Added optional closeButton prop to allow custom close button rendering
client/src/pages/platform/workflow-editor/WorkflowEditorLayout.tsx Refactored cluster elements dialog to always render WorkflowNodeDetailsPanel with custom close button that closes the entire canvas

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +184 to +193
<DialogClose asChild>
<Button
className="absolute right-2 top-2"
size="icon"
title="Close the canvas"
variant="ghost"
>
<XIcon />
</Button>
</DialogClose>
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The close button's behavior is potentially confusing. It's positioned in the WorkflowNodeDetailsPanel header (where users would expect it to close just the panel), but wrapping it in DialogClose makes it close the entire cluster elements canvas dialog instead.

Consider either:

  1. Using a separate close button for the canvas placed outside the panel header
  2. Implementing custom logic to close only the panel while keeping the canvas open
  3. Making it clear through positioning/styling that this button closes the entire canvas, not just the panel
Suggested change
<DialogClose asChild>
<Button
className="absolute right-2 top-2"
size="icon"
title="Close the canvas"
variant="ghost"
>
<XIcon />
</Button>
</DialogClose>
(() => {
const setPanelOpen = useWorkflowNodeDetailsPanelStore(state => state.setOpen);
return (
<Button
className="absolute right-2 top-2"
size="icon"
title="Close panel"
variant="ghost"
onClick={() => setPanelOpen(false)}
>
<XIcon />
</Button>
);
})()

Copilot uses AI. Check for mistakes.
/>
<>
<WorkflowNodeDetailsPanel
className="fixed inset-y-0 right-0 rounded-l-none"
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The border-none class was removed from the className prop. Since WorkflowNodeDetailsPanel has a default border (border border-stroke-neutral-secondary on line 1124), this panel will now display a border in the cluster elements canvas. If this is unintentional and you want to maintain the previous borderless appearance, add border-none back to the className: className="fixed inset-y-0 right-0 rounded-l-none border-none"

Suggested change
className="fixed inset-y-0 right-0 rounded-l-none"
className="fixed inset-y-0 right-0 rounded-l-none border-none"

Copilot uses AI. Check for mistakes.
updateWorkflowMutation={updateWorkflowMutation!}
workflowNodeOutputs={filteredWorkflowNodeOutputs ?? []}
/>
<>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sada je ovaj fragment <> visak

<WorkflowNodeDetailsPanel
className="fixed inset-y-0 right-0 rounded-l-none"
closeButton={
<DialogClose asChild>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivicac je li zelimo da se NodeDetailsPanel nikako ne moze zatvoriti -> da se automatski zatvara i canvas klikom na X ?

Jer onda bi tribali dodati i logiku kada se brise vec selektirani cluster element, da se u panelu mozda otvori mainRoot (trenutno brisanje selektiranog elementa samo zatvara panel) 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants