Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
</template>

<template #header>
<SearchBox v-model="searchQuery" size="lg" class="max-w-[384px]" />
<SearchBox
v-model="searchQuery"
size="lg"
class="max-w-[384px]"
autofocus
/>
</template>

<template #header-right-area>
Expand Down
10 changes: 8 additions & 2 deletions src/components/sidebar/tabs/SidebarTabTemplate.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<template>
<div
class="comfy-vue-side-bar-container group/sidebar-tab flex h-full flex-col"
:class="props.class"
:class="
cn(
'comfy-vue-side-bar-container group/sidebar-tab flex h-full flex-col',
props.class
)
"
>
<div class="comfy-vue-side-bar-header flex flex-col gap-2">
<Toolbar
Expand Down Expand Up @@ -35,6 +39,8 @@
import ScrollPanel from 'primevue/scrollpanel'
import Toolbar from 'primevue/toolbar'

import { cn } from '@/utils/tailwindUtil'

const props = defineProps<{
title: string
class?: string
Expand Down
16 changes: 13 additions & 3 deletions src/components/ui/TypeformPopoverButton.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { whenever } from '@vueuse/core'
import { breakpointsTailwind, useBreakpoints, whenever } from '@vueuse/core'
import { useTemplateRef } from 'vue'

import Popover from '@/components/ui/Popover.vue'
Expand All @@ -10,6 +10,7 @@ defineProps<{
}>()

const feedbackRef = useTemplateRef('feedbackRef')
const isMobile = useBreakpoints(breakpointsTailwind).smaller('md')

whenever(feedbackRef, () => {
const scriptEl = document.createElement('script')
Expand All @@ -18,9 +19,18 @@ whenever(feedbackRef, () => {
})
</script>
<template>
<Popover>
<a
v-if="isMobile"
:href="`https://form.typeform.com/to/${dataTfWidget}`"
target="_blank"
>
<Button variant="inverted" class="rounded-full size-12" v-bind="$attrs">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Option: You could do as='a' to render the button as an anchor. It'd accept the anchor properties too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Much better. Thanks.

<i class="icon-[lucide--circle-question-mark] size-6" />
</Button>
</a>
<Popover v-else>
<template #button>
<Button variant="inverted" class="rounded-full size-12">
<Button variant="inverted" class="rounded-full size-12" v-bind="$attrs">
<i class="icon-[lucide--circle-question-mark] size-6" />
</Button>
Comment thread
AustinMroz marked this conversation as resolved.
Outdated
</template>
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/extensions/linearMode/LinearControls.vue
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ defineExpose({ runButtonClick })
<NodeWidgets
:node-data
:style="{ background: applyLightThemeColor(nodeData.bgcolor) }"
class="py-3 gap-y-3 **:[.col-span-2]:grid-cols-1 not-has-[textarea]:flex-0 rounded-lg max-w-100"
class="py-3 gap-y-3 **:[.col-span-2]:grid-cols-1 *:has-[textarea]:h-50 rounded-lg max-w-100"
/>
</template>
</div>
Expand Down Expand Up @@ -237,7 +237,7 @@ defineExpose({ runButtonClick })
:node-data
:class="
cn(
'py-3 gap-y-3 **:[.col-span-2]:grid-cols-1 not-has-[textarea]:flex-0 rounded-lg',
'py-3 gap-y-3 **:[.col-span-2]:grid-cols-1 *:has-[textarea]:h-50 rounded-lg',
nodeData.hasErrors &&
'ring-2 ring-inset ring-node-stroke-error'
)
Expand Down
58 changes: 58 additions & 0 deletions src/renderer/extensions/linearMode/MobileMenu.vue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is pretty close to being a component... 😉

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For collapsible? I think too many of my styling tweaks are specific to this use case and still manage to leave some usability problems half solved. I'd rather return to this after I've had more time to think things over.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For a collapsible menu. I agree with your instincts.

Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<script setup lang="ts">
import {
CollapsibleRoot,
CollapsibleTrigger,
CollapsibleContent
} from 'reka-ui'

import WorkflowsSidebarTab from '@/components/sidebar/tabs/WorkflowsSidebarTab.vue'
import Button from '@/components/ui/button/Button.vue'
import Popover from '@/components/ui/Popover.vue'
import { useWorkflowTemplateSelectorDialog } from '@/composables/useWorkflowTemplateSelectorDialog'
import { t } from '@/i18n'
import { useCommandStore } from '@/stores/commandStore'
</script>
<template>
<CollapsibleRoot class="flex flex-col">
<CollapsibleTrigger as-child>
<Button variant="secondary" class="size-10 self-end m-4 mb-2">
<i class="icon-[lucide--menu] size-8" />
</Button>
Comment on lines +18 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add an accessible label to the icon-only menu trigger.
Screen readers won’t have a name for this control.

✅ Suggested fix
-      <Button variant="secondary" class="size-10 self-end m-4 mb-2">
+      <Button
+        variant="secondary"
+        class="size-10 self-end m-4 mb-2"
+        :aria-label="t('linearMode.openMenu')"
+      >
         <i class="icon-[lucide--menu] size-8" />
       </Button>

Based on learnings, “If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.”

🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/MobileMenu.vue` around lines 18 - 20, The
menu trigger Button in MobileMenu.vue is icon-only and lacks an accessible name;
add a clear aria-label (e.g., "Open menu" or "Open navigation menu") to the
Button (the <Button> element that currently contains the <i
class="icon-[lucide--menu] ...">) and ensure the icon <i> is marked decorative
(aria-hidden="true") if applicable so screen readers announce the aria-label
instead of the icon.

</CollapsibleTrigger>
<CollapsibleContent class="flex gap-2 flex-col">
<div class="w-full border-b-2 border-border-subtle" />
<Popover>
<template #button>
<Button variant="secondary" size="lg" class="w-full">
<i class="icon-[comfy--workflow]" />
{{ t('Workflows') }}
</Button>
</template>
<WorkflowsSidebarTab class="h-300 w-[80vw]" />
</Popover>
<Button
variant="secondary"
size="lg"
class="w-full"
@click="useWorkflowTemplateSelectorDialog().show('menu')"
>
<i class="icon-[comfy--template]" />
{{ t('sideToolbar.templates') }}
</Button>
<Button
variant="secondary"
size="lg"
class="w-full"
@click="
useCommandStore().execute('Comfy.ToggleLinear', {
metadata: { source: 'button' }
})
Comment on lines +33 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Hoist composables/stores out of the template handlers.
This avoids repeated calls and keeps logic centralized.

♻️ Suggested refactor
<script setup lang="ts">
 import { useWorkflowTemplateSelectorDialog } from '@/composables/useWorkflowTemplateSelectorDialog'
 import { t } from '@/i18n'
 import { useCommandStore } from '@/stores/commandStore'

+const workflowTemplateSelectorDialog = useWorkflowTemplateSelectorDialog()
+const commandStore = useCommandStore()
</script>
-        `@click`="useWorkflowTemplateSelectorDialog().show('menu')"
+        `@click`="workflowTemplateSelectorDialog.show('menu')"
-        `@click`="
-          useCommandStore().execute('Comfy.ToggleLinear', {
-            metadata: { source: 'button' }
-          })
-        "
+        `@click`="
+          commandStore.execute('Comfy.ToggleLinear', {
+            metadata: { source: 'button' }
+          })
+        "
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
variant="secondary"
size="lg"
class="w-full"
@click="useWorkflowTemplateSelectorDialog().show('menu')"
>
<i class="icon-[comfy--template]" />
{{ t('sideToolbar.templates') }}
</Button>
<Button
variant="secondary"
size="lg"
class="w-full"
@click="
useCommandStore().execute('Comfy.ToggleLinear', {
metadata: { source: 'button' }
})
<Button
variant="secondary"
size="lg"
class="w-full"
`@click`="workflowTemplateSelectorDialog.show('menu')"
>
<i class="icon-[comfy--template]" />
{{ t('sideToolbar.templates') }}
</Button>
<Button
variant="secondary"
size="lg"
class="w-full"
`@click`="
commandStore.execute('Comfy.ToggleLinear', {
metadata: { source: 'button' }
})
"
>
🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/MobileMenu.vue` around lines 33 - 49, The
template currently calls composables/stores inline
(useWorkflowTemplateSelectorDialog() and useCommandStore().execute(...)) which
causes repeated lookups; hoist these by calling
useWorkflowTemplateSelectorDialog and useCommandStore once in the component
setup, store their returned objects (e.g., templateDialog and commandStore) and
expose wrapper methods (e.g., openTemplateMenu -> templateDialog.show('menu')
and toggleLinear -> commandStore.execute('Comfy.ToggleLinear', { metadata: {
source: 'button' } })) so the template binds to these methods instead of calling
composables inline.

"
>
<i class="icon-[lucide--log-out]" />
{{ t('linearMode.graphMode') }}
</Button>
<div class="w-full border-b-2 border-border-subtle" />
</CollapsibleContent>
</CollapsibleRoot>
</template>
6 changes: 5 additions & 1 deletion src/renderer/extensions/linearMode/OutputHistory.vue
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ watch([selectedIndex, selectedOutput], doEmit)
watch(
() => outputs.media.value,
(newAssets, oldAssets) => {
if (newAssets.length === oldAssets.length || oldAssets.length === 0) return
if (
newAssets.length === oldAssets.length ||
(oldAssets.length === 0 && newAssets.length !== 1)
)
return
Comment thread
AustinMroz marked this conversation as resolved.
if (selectedIndex.value[0] <= 0) {
selectedIndex.value = [0, 0]
return
Expand Down
16 changes: 10 additions & 6 deletions src/views/LinearView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Splitter from 'primevue/splitter'
import SplitterPanel from 'primevue/splitterpanel'
import { ref, useTemplateRef } from 'vue'

import ModeToggle from '@/components/sidebar/ModeToggle.vue'
import TopbarBadges from '@/components/topbar/TopbarBadges.vue'
import WorkflowTabs from '@/components/topbar/WorkflowTabs.vue'
import TypeformPopoverButton from '@/components/ui/TypeformPopoverButton.vue'
Expand All @@ -17,6 +18,7 @@ import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
import { useSettingStore } from '@/platform/settings/settingStore'
import LinearControls from '@/renderer/extensions/linearMode/LinearControls.vue'
import LinearPreview from '@/renderer/extensions/linearMode/LinearPreview.vue'
import MobileMenu from '@/renderer/extensions/linearMode/MobileMenu.vue'
import OutputHistory from '@/renderer/extensions/linearMode/OutputHistory.vue'
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
import type { ResultItemImpl } from '@/stores/queueStore'
Expand Down Expand Up @@ -58,6 +60,7 @@ const linearWorkflowRef = useTemplateRef('linearWorkflowRef')
v-if="mobileDisplay"
class="justify-center border-border-subtle border-t overflow-y-scroll h-[calc(100%-38px)] bg-comfy-menu-bg"
>
<MobileMenu />
<div class="flex flex-col text-muted-foreground">
<LinearPreview
:latent-preview="
Expand All @@ -84,12 +87,13 @@ const linearWorkflowRef = useTemplateRef('linearWorkflowRef')
"
/>
<LinearControls ref="linearWorkflowRef" mobile />
<div class="text-base-foreground flex items-center gap-4 justify-end m-4">
<a
href="https://form.typeform.com/to/gmVqFi8l"
v-text="t('linearMode.beta')"
/>
<TypeformPopoverButton data-tf-widget="gmVqFi8l" />
<div class="text-base-foreground flex items-center gap-4">
<div class="border-r border-border-subtle">
<ModeToggle class="m-2" />
</div>
<div class="flex-1" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternative: ml-auto on the subsequent element. (I like to limit spacers to cases where I need multiple)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed (and neat)

<div v-text="t('linearMode.beta')" />
<TypeformPopoverButton data-tf-widget="gmVqFi8l" class="mx-2" />
</div>
</div>
<Splitter
Expand Down