Redesign image generation node with unified headers and Next.js Image migration #2051
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,78 +4,68 @@ import { | |||||||||||
| useWorkflowDesigner, | ||||||||||||
| } from "@giselle-sdk/giselle/react"; | ||||||||||||
| import clsx from "clsx/lite"; | ||||||||||||
| import { Maximize2 } from "lucide-react"; | ||||||||||||
| import { useCallback } from "react"; | ||||||||||||
| import { GenerateImageIcon } from "../../../icons"; | ||||||||||||
| import { EmptyState } from "../../../ui/empty-state"; | ||||||||||||
| import { GenerationView } from "../../../ui/generation-view"; | ||||||||||||
|
|
||||||||||||
| function Empty() { | ||||||||||||
| return ( | ||||||||||||
| <div className="relative bg-inverse/10 min-h-[250px] rounded-[8px] flex justify-center items-center text-black-400"> | ||||||||||||
| <EmptyState | ||||||||||||
| icon={<GenerateImageIcon width={24} height={24} />} | ||||||||||||
| title="Nothing generated yet." | ||||||||||||
| description="Generate or adjust the Prompt to see results." | ||||||||||||
| className="text-black-400" | ||||||||||||
| /> | ||||||||||||
| </div> | ||||||||||||
| ); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| export function GenerationPanel({ | ||||||||||||
| node, | ||||||||||||
| onClickGenerateButton, | ||||||||||||
| onExpand, | ||||||||||||
| isExpanded, | ||||||||||||
| }: { | ||||||||||||
| node: ImageGenerationNode; | ||||||||||||
| onClickGenerateButton?: () => void; | ||||||||||||
| onExpand?: () => void; | ||||||||||||
| isExpanded?: boolean; | ||||||||||||
| }) { | ||||||||||||
|
Comment on lines
26
to
36
Contributor
There was a problem hiding this comment. 🧩 Analysis chainVerify: Is the generate CTA intentionally removed from the empty state?
Also applies to: 49-51 🏁 Script executed: #!/bin/bash
# Find usages of GenerationPanel and check if onClickGenerateButton is still passed.
rg -nP --type=ts --type=tsx -C2 '\bGenerationPanel\s*\('
rg -nP --type=ts --type=tsx -C2 'onClickGenerateButton='Length of output: 55 </script> 🏁 Script executed: #!/bin/bash
# Search for GenerationPanel usages without --type flag
rg -nP '\bGenerationPanel\s*\(' -g '*.ts' -g '*.tsx' -C2
echo "---"
# Search for onClickGenerateButton parameter passing
rg -nP 'onClickGenerateButton=' -g '*.ts' -g '*.tsx' -C2
echo "---"
# Inspect the file to see lines 49-51
cat internal-packages/workflow-designer-ui/src/editor/properties-panel/image-generation-node-properties-panel/generation-panel.tsx | sed -n '1,60p'Length of output: 8156 </script> 🏁 Script executed: #!/bin/bash
# Get full file content to see where _handleGenerate is used
wc -l internal-packages/workflow-designer-ui/src/editor/properties-panel/image-generation-node-properties-panel/generation-panel.tsx
echo "---Full File---"
cat internal-packages/workflow-designer-ui/src/editor/properties-panel/image-generation-node-properties-panel/generation-panel.tsx
echo "---"
# Search for _handleGenerate usage
rg '_handleGenerate' internal-packages/workflow-designer-ui/src/editor/properties-panel/image-generation-node-properties-panel/generation-panel.tsx -n
echo "---"
# Check GenerationView component definition
fd GenerationView.tsx -g '*.tsx' | head -5Length of output: 3004 Remove unused The
🤖 Prompt for AI Agents |
||||||||||||
| const { data } = useWorkflowDesigner(); | ||||||||||||
| const { currentGeneration } = useNodeGenerations({ | ||||||||||||
| nodeId: node.id, | ||||||||||||
| origin: { type: "studio", workspaceId: data.id }, | ||||||||||||
| }); | ||||||||||||
|
|
||||||||||||
| const _handleGenerate = useCallback(() => { | ||||||||||||
| if (onClickGenerateButton) { | ||||||||||||
| onClickGenerateButton(); | ||||||||||||
| } | ||||||||||||
| }, [onClickGenerateButton]); | ||||||||||||
|
Comment on lines
+43
to
47
There was a problem hiding this comment. The _handleGenerate callback is defined but never used. This unused code should be removed.
Suggested change
|
||||||||||||
|
|
||||||||||||
| if (currentGeneration === undefined) { | ||||||||||||
| return <Empty />; | ||||||||||||
| } | ||||||||||||
| return ( | ||||||||||||
| <div | ||||||||||||
| className={clsx( | ||||||||||||
| "relative flex flex-col bg-inverse/10 rounded-[8px] py-[8px]", | ||||||||||||
| isExpanded ? "flex-1 min-h-0" : "min-h-[250px]", | ||||||||||||
| )} | ||||||||||||
| > | ||||||||||||
| {onExpand && ( | ||||||||||||
| <button | ||||||||||||
| type="button" | ||||||||||||
| onClick={onExpand} | ||||||||||||
| className="absolute bottom-[8px] right-[8px] size-[32px] rounded-full bg-inverse/10 hover:bg-inverse/20 flex items-center justify-center transition-colors group z-10" | ||||||||||||
| aria-label="Expand" | ||||||||||||
| > | ||||||||||||
| <Maximize2 className="size-[16px] text-inverse group-hover:text-inverse/80" /> | ||||||||||||
| </button> | ||||||||||||
| )} | ||||||||||||
| <div | ||||||||||||
| className={clsx( | ||||||||||||
| "border-b border-white-400/20 py-[4px] px-[16px] flex items-center gap-[8px]", | ||||||||||||
|
|
||||||||||||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
disabledattribute instead ofaria-disabledfor better accessibility.The current implementation uses
aria-disabledwhile still keeping the button focusable and in the keyboard tab order. WhenisDisabledis true, keyboard users can focus the button but activation does nothing, creating a confusing experience.According to WAI-ARIA best practices,
aria-disabledshould only be used when an element needs to remain in the tab order for context. For truly non-interactive items, the HTMLdisabledattribute is more appropriate as it removes the element from the tab order entirely.Apply this diff to use the native
disabledattribute:{group.models.map((m) => { const isDisabled = Boolean(m.disabled); return ( <button key={`${group.provider}-${m.id}`} type="button" + disabled={isDisabled} onClick={() => { - if (isDisabled) return; onSelect(group.provider, m.id); setOpen(false); }} - aria-disabled={isDisabled} className={clsx( "flex gap-[12px] items-center p-[4px] rounded-[4px] text-left", isDisabled - ? "opacity-50 cursor-not-allowed" - : "hover:bg-white/5 focus:bg-white/5 cursor-pointer", + ? "opacity-50 cursor-not-allowed disabled:pointer-events-none" + : "hover:bg-white/5 focus:bg-white/5 cursor-pointer enabled:hover:bg-white/5", )} >Note: If you need disabled items to remain in tab order for specific UX reasons, please document that decision and ensure proper keyboard event handling (Enter/Space keys) is implemented.
📝 Committable suggestion
🤖 Prompt for AI Agents