-
Notifications
You must be signed in to change notification settings - Fork 644
[DO NOT MERGE] Cloud/1.37 - cloud release branch #8063
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
Changes from 10 commits
c77f0cb
5ec29f6
3c99e75
cd6047f
d85c469
9132f87
a3cd630
40b0954
5a276f2
ac6adb0
3eb8c6a
e036d76
a55cae5
05cbcce
995906a
def9b55
b7ddd50
f074243
abb2b15
751253f
14a2208
06bc103
a6da367
32ce523
b8a103b
7faf8e0
0c3d569
5d94c11
2d0f3d6
eb7d0c7
b98d53e
88f7886
9db2fd8
9b7f20c
7a5fb57
12761f8
4c3c61f
113a6a7
745ea0a
ec91aa8
3eb15bb
3b7e102
1963f28
e82c692
e751cf4
a5f50ac
4c2edae
8e5a037
72add79
2f9ede1
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,28 @@ | ||
| <script setup lang="ts"> | ||
| import Button from '@/components/ui/button/Button.vue' | ||
| import { t } from '@/i18n' | ||
| import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' | ||
| import { useCommandStore } from '@/stores/commandStore' | ||
|
|
||
| const canvasStore = useCanvasStore() | ||
| </script> | ||
| <template> | ||
| <div class="p-1 bg-secondary-background rounded-lg w-10"> | ||
| <Button | ||
| size="icon" | ||
| :title="t('linearMode.linearMode')" | ||
| :variant="canvasStore.linearMode ? 'inverted' : 'secondary'" | ||
| @click="useCommandStore().execute('Comfy.ToggleLinear')" | ||
| > | ||
| <i class="icon-[lucide--panels-top-left]" /> | ||
| </Button> | ||
| <Button | ||
| size="icon" | ||
| :title="t('linearMode.graphMode')" | ||
| :variant="canvasStore.linearMode ? 'secondary' : 'inverted'" | ||
| @click="useCommandStore().execute('Comfy.ToggleLinear')" | ||
| > | ||
| <i class="icon-[comfy--workflow]" /> | ||
| </Button> | ||
| </div> | ||
|
Comment on lines
+1
to
+40
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. 🧩 Analysis chain🏁 Script executed: rg -n "linearMode" src/locales/ -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend Length of output: 3470 🏁 Script executed: # Check how t() is imported and used in other components
rg "from '@/i18n'" src/components/ | head -20Repository: Comfy-Org/ComfyUI_frontend Length of output: 1377 🏁 Script executed: # Check if useI18n is used elsewhere
rg "useI18n" src/components/ | head -20Repository: Comfy-Org/ComfyUI_frontend Length of output: 1552 Use While the i18n keys Replace the direct import The rest of the component implementation is clean: proper use of Composition API, no unnecessary style blocks, correct command binding, and clear toggle logic. 🤖 Prompt for AI Agents |
||
| </template> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,10 @@ | |
| > | ||
| <i class="pi pi-bars" /> | ||
| </Button> | ||
| <i | ||
| v-else-if="workflowOption.workflow.activeState?.extra?.linearMode" | ||
| class="icon-[lucide--panels-top-left] bg-primary-background" | ||
| /> | ||
|
Comment on lines
+10
to
+13
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. 🧩 Analysis chain🏁 Script executed: rg -n "linearMode" src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend Length of output: 130 🏁 Script executed: cat -n src/components/topbar/WorkflowTab.vue | head -100Repository: Comfy-Org/ComfyUI_frontend Length of output: 3677 🏁 Script executed: cat -n src/components/topbar/WorkflowTab.vue | tail -150Repository: Comfy-Org/ComfyUI_frontend Length of output: 5359 Improve icon accessibility with title attribute, but fix aria-hidden value. The icon is purely decorative and lacks an accessible label. Add a 🔧 Suggested fix <i
v-else-if="workflowOption.workflow.activeState?.extra?.linearMode"
class="icon-[lucide--panels-top-left] bg-primary-background"
+ :title="t('linearMode.linearMode')"
+ aria-hidden="true"
/>🤖 Prompt for AI Agents |
||
| <span class="workflow-label inline-block max-w-[150px] truncate text-sm"> | ||
| {{ workflowOption.workflow.filename }} | ||
| </span> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,76 @@ | ||||||||||||||||||||||||||||||
| <script setup lang="ts"> | ||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||
| PopoverArrow, | ||||||||||||||||||||||||||||||
| PopoverContent, | ||||||||||||||||||||||||||||||
| PopoverPortal, | ||||||||||||||||||||||||||||||
| PopoverRoot, | ||||||||||||||||||||||||||||||
| PopoverTrigger | ||||||||||||||||||||||||||||||
| } from 'reka-ui' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import Button from '@/components/ui/button/Button.vue' | ||||||||||||||||||||||||||||||
| import { cn } from '@/utils/tailwindUtil' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| defineOptions({ | ||||||||||||||||||||||||||||||
| inheritAttrs: false | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| defineProps<{ | ||||||||||||||||||||||||||||||
| entries?: { label: string; action?: () => void; icon?: string }[][] | ||||||||||||||||||||||||||||||
| icon?: string | ||||||||||||||||||||||||||||||
| to?: string | HTMLElement | ||||||||||||||||||||||||||||||
| }>() | ||||||||||||||||||||||||||||||
| </script> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| <template> | ||||||||||||||||||||||||||||||
| <PopoverRoot v-slot="{ close }"> | ||||||||||||||||||||||||||||||
| <PopoverTrigger as-child> | ||||||||||||||||||||||||||||||
| <slot name="button"> | ||||||||||||||||||||||||||||||
| <Button size="icon"> | ||||||||||||||||||||||||||||||
| <i :class="icon ?? 'icon-[lucide--ellipsis]'" /> | ||||||||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||||||||
| </slot> | ||||||||||||||||||||||||||||||
| </PopoverTrigger> | ||||||||||||||||||||||||||||||
|
Comment on lines
+26
to
+32
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. Add aria-label for icon-only button accessibility. The default trigger button contains only an icon without visible text. Per accessibility guidelines, icon-only buttons need an ♻️ Suggested fix <slot name="button">
- <Button size="icon">
+ <Button size="icon" aria-label="Open menu">
<i :class="icon ?? 'icon-[lucide--ellipsis]'" />
</Button>
</slot>Note: Consider using vue-i18n for the aria-label string if this component is user-facing. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| <PopoverPortal :to> | ||||||||||||||||||||||||||||||
| <PopoverContent | ||||||||||||||||||||||||||||||
| side="bottom" | ||||||||||||||||||||||||||||||
| :side-offset="5" | ||||||||||||||||||||||||||||||
| :collision-padding="10" | ||||||||||||||||||||||||||||||
| v-bind="$attrs" | ||||||||||||||||||||||||||||||
| class="rounded-lg p-2 bg-base-background shadow-sm border border-border-subtle will-change-[transform,opacity] data-[state=open]:data-[side=top]:animate-slideDownAndFade data-[state=open]:data-[side=right]:animate-slideLeftAndFade data-[state=open]:data-[side=bottom]:animate-slideUpAndFade data-[state=open]:data-[side=left]:animate-slideRightAndFade" | ||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||
| <slot> | ||||||||||||||||||||||||||||||
| <div class="flex flex-col p-1"> | ||||||||||||||||||||||||||||||
| <section | ||||||||||||||||||||||||||||||
| v-for="(entryGroup, index) in entries ?? []" | ||||||||||||||||||||||||||||||
| :key="index" | ||||||||||||||||||||||||||||||
| class="flex flex-col border-b-2 last:border-none border-border-subtle" | ||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||||||
| v-for="{ label, action, icon } in entryGroup" | ||||||||||||||||||||||||||||||
| :key="label" | ||||||||||||||||||||||||||||||
| :class=" | ||||||||||||||||||||||||||||||
| cn( | ||||||||||||||||||||||||||||||
| 'flex flex-row gap-4 p-2 rounded-sm my-1', | ||||||||||||||||||||||||||||||
| action && | ||||||||||||||||||||||||||||||
| 'cursor-pointer hover:bg-secondary-background-hover' | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| " | ||||||||||||||||||||||||||||||
| @click=" | ||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||
| if (!action) return | ||||||||||||||||||||||||||||||
| action() | ||||||||||||||||||||||||||||||
| close() | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| " | ||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||
| <i v-if="icon" :class="icon" /> | ||||||||||||||||||||||||||||||
| {{ label }} | ||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||
| </section> | ||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||
| </slot> | ||||||||||||||||||||||||||||||
| <PopoverArrow class="fill-base-background stroke-border-subtle" /> | ||||||||||||||||||||||||||||||
| </PopoverContent> | ||||||||||||||||||||||||||||||
| </PopoverPortal> | ||||||||||||||||||||||||||||||
| </PopoverRoot> | ||||||||||||||||||||||||||||||
| </template> | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||
| <script setup lang="ts"> | ||||||||||||||||||||||||||||||
| import { whenever } from '@vueuse/core' | ||||||||||||||||||||||||||||||
| import { useTemplateRef } from 'vue' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import Popover from '@/components/ui/Popover.vue' | ||||||||||||||||||||||||||||||
| import Button from '@/components/ui/button/Button.vue' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| defineProps<{ | ||||||||||||||||||||||||||||||
| dataTfWidget: string | ||||||||||||||||||||||||||||||
| }>() | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const feedbackRef = useTemplateRef('feedbackRef') | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| whenever(feedbackRef, () => { | ||||||||||||||||||||||||||||||
| const scriptEl = document.createElement('script') | ||||||||||||||||||||||||||||||
| scriptEl.src = '//embed.typeform.com/next/embed.js' | ||||||||||||||||||||||||||||||
| feedbackRef.value?.appendChild(scriptEl) | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+18
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. Potential multiple script injections on repeated popover opens. The ♻️ Suggested fix+const scriptInjected = ref(false)
+
whenever(feedbackRef, () => {
+ if (scriptInjected.value) return
const scriptEl = document.createElement('script')
scriptEl.src = '//embed.typeform.com/next/embed.js'
feedbackRef.value?.appendChild(scriptEl)
+ scriptInjected.value = true
})📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| </script> | ||||||||||||||||||||||||||||||
| <template> | ||||||||||||||||||||||||||||||
| <Popover> | ||||||||||||||||||||||||||||||
| <template #button> | ||||||||||||||||||||||||||||||
| <Button variant="inverted" class="rounded-full size-12"> | ||||||||||||||||||||||||||||||
| <i class="icon-[lucide--circle-question-mark] size-6" /> | ||||||||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||||||||
| </template> | ||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+26
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. Add aria-label for icon-only help button. The button contains only an icon without visible text. For screen reader accessibility, add an aria-label. As per coding guidelines, use vue-i18n for the label. ♻️ Suggested fix+<script setup lang="ts">
+import { useI18n } from 'vue-i18n'
+// ... existing imports
+
+const { t } = useI18n()
+// ... rest of script
+</script>
+
<template `#button`>
- <Button variant="inverted" class="rounded-full size-12">
+ <Button variant="inverted" class="rounded-full size-12" :aria-label="t('feedback.openFeedback')">
<i class="icon-[lucide--circle-question-mark] size-6" />
</Button>
</template>🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| <div ref="feedbackRef" data-tf-auto-resize :data-tf-widget /> | ||||||||||||||||||||||||||||||
| </Popover> | ||||||||||||||||||||||||||||||
| </template> | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| <script setup lang="ts"> | ||
| import { computed, ref, useTemplateRef } from 'vue' | ||
|
|
||
| const zoomPane = useTemplateRef('zoomPane') | ||
|
|
||
| const zoom = ref(1.0) | ||
| const panX = ref(0.0) | ||
| const panY = ref(0.0) | ||
|
|
||
| function handleWheel(e: WheelEvent) { | ||
| const zoomPaneEl = zoomPane.value | ||
| if (!zoomPaneEl) return | ||
|
|
||
| zoom.value -= e.deltaY | ||
| const { x, y, width, height } = zoomPaneEl.getBoundingClientRect() | ||
| const offsetX = e.clientX - x - width / 2 | ||
| const offsetY = e.clientY - y - height / 2 | ||
| const scaler = 1.1 ** (e.deltaY / -30) | ||
|
|
||
| panY.value = panY.value * scaler - offsetY * (scaler - 1) | ||
| panX.value = panX.value * scaler - offsetX * (scaler - 1) | ||
| } | ||
|
Comment on lines
+10
to
+22
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. Zoom calculation produces erratic behavior across devices. Line 14 applies raw 🐛 Proposed fix: Normalize deltaY and use consistent scaling function handleWheel(e: WheelEvent) {
const zoomPaneEl = zoomPane.value
if (!zoomPaneEl) return
- zoom.value -= e.deltaY
+ // Normalize deltaY to consistent step size
+ const delta = Math.sign(e.deltaY) * Math.min(Math.abs(e.deltaY), 100)
+ zoom.value -= delta
const { x, y, width, height } = zoomPaneEl.getBoundingClientRect()
const offsetX = e.clientX - x - width / 2
const offsetY = e.clientY - y - height / 2
- const scaler = 1.1 ** (e.deltaY / -30)
+ const scaler = 1.1 ** (delta / -30)
panY.value = panY.value * scaler - offsetY * (scaler - 1)
panX.value = panX.value * scaler - offsetX * (scaler - 1)
}🤖 Prompt for AI Agents |
||
|
|
||
| let dragging = false | ||
| function handleDown(e: PointerEvent) { | ||
| if (e.button !== 0) return | ||
|
|
||
| const zoomPaneEl = zoomPane.value | ||
| if (!zoomPaneEl) return | ||
| zoomPaneEl.parentElement?.focus() | ||
|
|
||
| zoomPaneEl.setPointerCapture(e.pointerId) | ||
| dragging = true | ||
| } | ||
| function handleMove(e: PointerEvent) { | ||
| if (!dragging) return | ||
| panX.value += e.movementX | ||
| panY.value += e.movementY | ||
|
Comment on lines
+24
to
+38
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. Shared The 🐛 Proposed fix: Use ref for instance isolation+const dragging = ref(false)
-let dragging = false
function handleDown(e: PointerEvent) {
if (e.button !== 0) return
const zoomPaneEl = zoomPane.value
if (!zoomPaneEl) return
zoomPaneEl.parentElement?.focus()
zoomPaneEl.setPointerCapture(e.pointerId)
- dragging = true
+ dragging.value = true
}
function handleMove(e: PointerEvent) {
- if (!dragging) return
+ if (!dragging.value) return
panX.value += e.movementX
panY.value += e.movementY
}And update template events: - `@pointerup`="dragging = false"
- `@pointercancel`="dragging = false"
+ `@pointerup`="dragging = false"
+ `@pointercancel`="dragging = false"🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| const transform = computed(() => { | ||
| const scale = 1.1 ** (zoom.value / 30) | ||
| const matrix = [scale, 0, 0, scale, panX.value, panY.value] | ||
| return `matrix(${matrix.join(',')})` | ||
| }) | ||
| </script> | ||
| <template> | ||
| <div | ||
| ref="zoomPane" | ||
| class="contain-size flex place-content-center" | ||
| @wheel="handleWheel" | ||
| @pointerdown.prevent="handleDown" | ||
| @pointermove="handleMove" | ||
| @pointerup="dragging = false" | ||
| @pointercancel="dragging = false" | ||
| > | ||
| <slot :style="{ transform }" /> | ||
| </div> | ||
| </template> | ||
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.
🧹 Nitpick | 🔵 Trivial
Add
aria-labelto icon-only buttons for accessibility.These buttons have no visible text—only icons. Screen reader users need aria-label to understand the button's purpose. The tooltip alone doesn't provide accessible labeling.
♿ Proposed accessibility fix
<Button v-tooltip="{ value: t('linearMode.linearMode'), showDelay: 300, hideDelay: 300 }" size="icon" :variant="canvasStore.linearMode ? 'inverted' : 'secondary'" + :aria-label="t('linearMode.linearMode')" `@click`="toggleLinearMode" > <i class="icon-[lucide--panels-top-left]" /> </Button> <Button v-tooltip="{ value: t('linearMode.graphMode'), showDelay: 300, hideDelay: 300 }" size="icon" :variant="canvasStore.linearMode ? 'secondary' : 'inverted'" + :aria-label="t('linearMode.graphMode')" `@click`="toggleLinearMode" > <i class="icon-[comfy--workflow]" /> </Button>Based on learnings: "Use aria-label only for elements without visible labels (e.g., icon-only buttons)."
🤖 Prompt for AI Agents