-
Notifications
You must be signed in to change notification settings - Fork 461
feat: Add visual crop preview widget for ImageCrop node - widget ImageCrop #7825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,147 @@ | ||||||||||||||
| <template> | ||||||||||||||
| <div | ||||||||||||||
| class="widget-expands relative flex h-full w-full flex-col gap-1" | ||||||||||||||
| @pointerdown.stop | ||||||||||||||
| @pointermove.stop | ||||||||||||||
| @pointerup.stop | ||||||||||||||
| > | ||||||||||||||
| <!-- Number inputs row --> | ||||||||||||||
| <div class="flex shrink-0 gap-1 px-1"> | ||||||||||||||
| <div class="flex flex-1 items-center gap-1"> | ||||||||||||||
| <label class="w-6 text-xs text-muted">X</label> | ||||||||||||||
| <input | ||||||||||||||
| v-model.number="cropX" | ||||||||||||||
| type="number" | ||||||||||||||
| :min="0" | ||||||||||||||
| class="h-6 w-full rounded border border-border bg-input px-1 text-xs" | ||||||||||||||
| @change="handleInputChange" | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| <div class="flex flex-1 items-center gap-1"> | ||||||||||||||
| <label class="w-6 text-xs text-muted">Y</label> | ||||||||||||||
| <input | ||||||||||||||
| v-model.number="cropY" | ||||||||||||||
| type="number" | ||||||||||||||
| :min="0" | ||||||||||||||
| class="h-6 w-full rounded border border-border bg-input px-1 text-xs" | ||||||||||||||
| @change="handleInputChange" | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| <div class="flex flex-1 items-center gap-1"> | ||||||||||||||
| <label class="w-6 text-xs text-muted">W</label> | ||||||||||||||
| <input | ||||||||||||||
| v-model.number="cropWidth" | ||||||||||||||
| type="number" | ||||||||||||||
| :min="1" | ||||||||||||||
| class="h-6 w-full rounded border border-border bg-input px-1 text-xs" | ||||||||||||||
| @change="handleInputChange" | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| <div class="flex flex-1 items-center gap-1"> | ||||||||||||||
| <label class="w-6 text-xs text-muted">H</label> | ||||||||||||||
| <input | ||||||||||||||
| v-model.number="cropHeight" | ||||||||||||||
| type="number" | ||||||||||||||
| :min="1" | ||||||||||||||
| class="h-6 w-full rounded border border-border bg-input px-1 text-xs" | ||||||||||||||
| @change="handleInputChange" | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
|
|
||||||||||||||
| <!-- Image preview container --> | ||||||||||||||
| <div | ||||||||||||||
| ref="containerEl" | ||||||||||||||
| class="relative min-h-0 flex-1 overflow-hidden rounded-[5px] bg-node-component-surface" | ||||||||||||||
| > | ||||||||||||||
| <div v-if="isLoading" class="flex size-full items-center justify-center"> | ||||||||||||||
| <span class="text-sm">{{ $t('imageCrop.loading') }}</span> | ||||||||||||||
| </div> | ||||||||||||||
|
|
||||||||||||||
| <div | ||||||||||||||
| v-else-if="!imageUrl" | ||||||||||||||
| class="flex size-full flex-col items-center justify-center text-center" | ||||||||||||||
| > | ||||||||||||||
| <i class="mb-2 icon-[lucide--image] h-12 w-12" /> | ||||||||||||||
| <p class="text-sm">{{ $t('imageCrop.noInputImage') }}</p> | ||||||||||||||
| </div> | ||||||||||||||
|
|
||||||||||||||
| <img | ||||||||||||||
| v-else | ||||||||||||||
| ref="imageEl" | ||||||||||||||
| :src="imageUrl" | ||||||||||||||
| :alt="$t('imageCrop.cropPreviewAlt')" | ||||||||||||||
| draggable="false" | ||||||||||||||
| class="block size-full object-contain select-none brightness-50" | ||||||||||||||
| @load="handleImageLoad" | ||||||||||||||
| @error="handleImageError" | ||||||||||||||
| @dragstart.prevent | ||||||||||||||
| /> | ||||||||||||||
|
|
||||||||||||||
| <div | ||||||||||||||
| v-if="imageUrl && !isLoading" | ||||||||||||||
| class="absolute box-border cursor-move overflow-hidden border-2 border-white" | ||||||||||||||
| :style="cropBoxStyle" | ||||||||||||||
| @pointerdown="handleDragStart" | ||||||||||||||
| @pointermove="handleDragMove" | ||||||||||||||
| @pointerup="handleDragEnd" | ||||||||||||||
| > | ||||||||||||||
| <div class="pointer-events-none size-full" :style="cropImageStyle" /> | ||||||||||||||
| </div> | ||||||||||||||
jtydhr88 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
| <div | ||||||||||||||
| v-for="handle in resizeHandles" | ||||||||||||||
| v-show="imageUrl && !isLoading" | ||||||||||||||
| :key="handle.direction" | ||||||||||||||
| :class="['absolute', handle.class]" | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Use cn() utility instead of array class syntax. Per coding guidelines, use the 🔎 Proposed refactorImport the utility at the top of the script: +import { cn } from '@/utils/tailwindUtil'Then update the binding: - :class="['absolute', handle.class]"
+ :class="cn('absolute', handle.class)"Based on coding guidelines. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| :style="handle.style" | ||||||||||||||
| @pointerdown="(e) => handleResizeStart(e, handle.direction)" | ||||||||||||||
| @pointermove="handleResizeMove" | ||||||||||||||
| @pointerup="handleResizeEnd" | ||||||||||||||
| /> | ||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| </template> | ||||||||||||||
|
|
||||||||||||||
| <script setup lang="ts"> | ||||||||||||||
| import { useTemplateRef } from 'vue' | ||||||||||||||
|
|
||||||||||||||
| import { useImageCrop } from '@/composables/useImageCrop' | ||||||||||||||
| import type { CropRegionValue } from '@/lib/litegraph/src/types/widgets' | ||||||||||||||
| import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema' | ||||||||||||||
|
|
||||||||||||||
| const props = defineProps<{ | ||||||||||||||
| nodeId: NodeId | ||||||||||||||
| }>() | ||||||||||||||
|
Comment on lines
+113
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Use Vue 3.5 style props destructuring. Per coding guidelines, prefer reactive props destructuring in Vue 3.5+. 🔎 Proposed refactor-const props = defineProps<{
+const { nodeId } = defineProps<{
nodeId: NodeId
}>()
...
-} = useImageCrop(props.nodeId, { imageEl, containerEl, modelValue })
+} = useImageCrop(nodeId, { imageEl, containerEl, modelValue })Based on learnings and coding guidelines. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| const modelValue = defineModel<CropRegionValue>({ | ||||||||||||||
| default: () => ({ x: 0, y: 0, width: 512, height: 512 }) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| const imageEl = useTemplateRef<HTMLImageElement>('imageEl') | ||||||||||||||
| const containerEl = useTemplateRef<HTMLDivElement>('containerEl') | ||||||||||||||
|
|
||||||||||||||
| const { | ||||||||||||||
| imageUrl, | ||||||||||||||
| isLoading, | ||||||||||||||
|
|
||||||||||||||
| cropX, | ||||||||||||||
| cropY, | ||||||||||||||
| cropWidth, | ||||||||||||||
| cropHeight, | ||||||||||||||
|
|
||||||||||||||
| cropBoxStyle, | ||||||||||||||
| cropImageStyle, | ||||||||||||||
| resizeHandles, | ||||||||||||||
|
|
||||||||||||||
| handleImageLoad, | ||||||||||||||
| handleImageError, | ||||||||||||||
| handleInputChange, | ||||||||||||||
| handleDragStart, | ||||||||||||||
| handleDragMove, | ||||||||||||||
| handleDragEnd, | ||||||||||||||
| handleResizeStart, | ||||||||||||||
| handleResizeMove, | ||||||||||||||
| handleResizeEnd | ||||||||||||||
| } = useImageCrop(props.nodeId, { imageEl, containerEl, modelValue }) | ||||||||||||||
| </script> | ||||||||||||||
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.
Improve accessibility by associating labels with inputs.
The
<label>elements are not programmatically associated with their corresponding inputs. Useidandforattributes or wrap inputs in labels for proper accessibility.🔎 Proposed fix for one input (apply pattern to all)
Apply the same pattern for Y (
id="crop-y"), W (id="crop-width"), and H (id="crop-height").📝 Committable suggestion