Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
72 changes: 63 additions & 9 deletions browser_tests/tests/gettingStartedTour.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const CARD_TESTID_PREFIX = 'getting-started-card-'

/** The prompt id the tour's run is queued under, so WS events can address it. */
const TOUR_JOB_ID = 'first-run-tour-prompt'
const LINKED_TEMPLATE_ID: SupportedTemplateId = 'image_z_image_turbo'

/** A prompt the queue accepts, so the walk does not depend on the backend's models. */
const QUEUED_PROMPT: PromptResponse = {
Expand Down Expand Up @@ -65,26 +66,41 @@ async function tourLength(card: Locator): Promise<number> {
return Number(/Step \d+ of (\d+)/.exec(label ?? '')?.[1])
}

async function clearWorkflowHistory(page: Page) {
await page.evaluate(() => {
for (const key of Object.keys(localStorage))
if (key.startsWith('Comfy.Workflow.')) localStorage.removeItem(key)
})
}

/**
* The grid backfills whichever curated templates a backend does not serve, so
* the walk tours a card that is actually on screen rather than a fixed id.
*/
async function firstPinnedTemplateOnScreen(
page: Page
): Promise<SupportedTemplateId> {
const testIds = await page
.locator(`[data-testid^="${CARD_TESTID_PREFIX}"]`)
.evaluateAll((cards) =>
cards.map((card) => card.getAttribute('data-testid') ?? '')
const cardTestIds = () =>
page
.locator(`[data-testid^="${CARD_TESTID_PREFIX}"]`)
.evaluateAll((cards) =>
cards.map((card) => card.getAttribute('data-testid') ?? '')
)

await expect
.poll(
async () =>
(await cardTestIds()).some((testId) =>
isPinned(testId.slice(CARD_TESTID_PREFIX.length))
),
{ message: 'the Getting Started grid never rendered a pinned template' }
)
const templateId = testIds
.toBe(true)

const templateId = (await cardTestIds())
.map((testId) => testId.slice(CARD_TESTID_PREFIX.length))
.find(isPinned)

expect(
templateId,
`this backend serves none of the pinned templates: ${testIds.join(', ')}`
).toBeDefined()
return templateId!
}

Expand Down Expand Up @@ -263,4 +279,42 @@ test.describe('First-run tour', { tag: ['@cloud', '@ui'] }, () => {
).toBeHidden()
await expect(page.getByTestId('coach-spotlight')).toBeHidden()
})

test.describe('arriving on a template link', () => {
test.beforeEach(async ({ comfyPage }) => {
await clearWorkflowHistory(comfyPage.page)
await comfyPage.setup({
clearStorage: false,
url: `/?template=${LINKED_TEMPLATE_ID}`
})
})

test('tours the template the link loaded', async ({ comfyPage }) => {
const { page } = comfyPage

await expect(
page.getByRole('dialog', { name: GETTING_STARTED_TITLE }),
'the link already chose a workflow, so there is nothing to choose'
).toBeHidden()
await expect(page.getByTestId('coach-spotlight')).toBeVisible()
await expect(page.getByTestId('coach-card')).toContainText('Step 1 of')
})
})

test.describe('arriving on a link that loads nothing', () => {
test.beforeEach(async ({ comfyPage }) => {
await clearWorkflowHistory(comfyPage.page)
await comfyPage.setup({
clearStorage: false,
url: '/?template=no_such_template_exists'
})
})

test('offers no tour', async ({ comfyPage }) => {
await expect(
comfyPage.page.getByTestId('coach-spotlight'),
'touring a graph the user never asked for is worse than no tour'
).toBeHidden()
})
})
})
22 changes: 19 additions & 3 deletions src/components/graph/GraphCanvas.vue
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
import TransformPane from '@/renderer/core/layout/transform/TransformPane.vue'
import type { StartupOutcome } from '@/platform/workflow/persistence/base/draftTypes'
import { useFirstRunEntry } from '@/renderer/extensions/firstRunTour/gettingStarted/firstRunEntry'
import MiniMap from '@/renderer/extensions/minimap/MiniMap.vue'
import LGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue'
Expand Down Expand Up @@ -505,6 +506,8 @@ useEventListener(
onMounted(async () => {
comfyApp.vueAppReady = true
workspaceStore.spinner = true
let startupOutcome: StartupOutcome | undefined
let urlTemplateId: string | undefined
try {
// ChangeTracker needs to be initialized before setup, as it will overwrite
// some listeners of litegraph canvas.
Expand Down Expand Up @@ -560,14 +563,15 @@ onMounted(async () => {
)

// Restore saved workflow and workflow tabs state
const startupOutcome = await workflowPersistence.initializeWorkflow()
startupOutcome = await workflowPersistence.initializeWorkflow()
await workflowPersistence.restoreWorkflowTabsState()
await workflowPersistence.loadTemplateFromUrlIfPresent()
urlTemplateId = await workflowPersistence.loadTemplateFromUrlIfPresent()
await useFirstRunEntry().handleStartupOutcome(startupOutcome)
} finally {
workspaceStore.spinner = false
}
await workflowPersistence.loadSharedWorkflowFromUrlIfPresent()
const sharedStatus =
await workflowPersistence.loadSharedWorkflowFromUrlIfPresent()

comfyApp.canvas.onSelectionChange = useChainCallback(
comfyApp.canvas.onSelectionChange,
Expand All @@ -584,6 +588,18 @@ onMounted(async () => {
void releaseStore.initialize()

emit('ready')

// The tour draws into an overlay that only mounts once `ready` has flushed.
await nextTick()
try {
await useFirstRunEntry().handleUrlWorkflow(
startupOutcome,
urlTemplateId,
sharedStatus
)
} catch (error) {
console.error('[GraphCanvas] Failed to offer the first-run tour:', error)
}
})

onUnmounted(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ export function useWorkflowPersistenceV2() {
const query = await ensureTemplateQueryFromIntent()
const hasTemplateUrl = query.template && typeof query.template === 'string'

if (hasTemplateUrl) {
await templateUrlLoader.loadTemplateFromUrl()
}
return hasTemplateUrl
? await templateUrlLoader.loadTemplateFromUrl()
: undefined
}

const loadSharedWorkflowFromUrlIfPresent = async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { app } from '@/scripts/app'
import { useDialogService } from '@/services/dialogService'
import { useDialogStore } from '@/stores/dialogStore'

type SharedWorkflowUrlLoadStatus =
export type SharedWorkflowUrlLoadStatus =
| 'not-present'
| 'loaded'
| 'loaded-without-assets'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ export function useTemplateUrlLoader() {
/**
* Loads template from URL query parameters if present
* Handles errors internally and shows appropriate user feedback
* @returns the id of the template that loaded, if one did
*/
const loadTemplateFromUrl = async () => {
const loadTemplateFromUrl = async (): Promise<string | undefined> => {
const templateParam = route.query.template

if (!templateParam || typeof templateParam !== 'string') {
Expand Down Expand Up @@ -121,11 +122,15 @@ export function useTemplateUrlLoader() {
templateName: templateParam
})
})
} else if (modeParam === 'linear') {
return
}

if (modeParam === 'linear') {
// Set linear mode after successful template load
useTelemetry()?.trackEnterLinear({ source: 'template_url' })
canvasStore.linearMode = true
}
return sourceParam === 'default' ? templateParam : undefined
} catch (error) {
console.error(
'[useTemplateUrlLoader] Failed to load template from URL:',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
/>
<div
aria-hidden="true"
class="pointer-events-none absolute inset-0 bg-linear-to-b from-black/40 via-transparent via-50% to-black/40 transition-opacity duration-300 ease-out group-hover/card:opacity-60"
class="pointer-events-none absolute inset-0 bg-linear-to-b from-black/60 via-black/20 via-50% to-black/70 transition-opacity duration-300 ease-out group-hover/card:opacity-0"
/>
<div
v-if="badgeIcon"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ const mocks = vi.hoisted(() => ({
tourFlag: true,
execute: vi.fn(),
settings: {} as Record<string, unknown>,
setSetting: vi.fn()
setSetting: vi.fn(),
beginTour: vi.fn()
}))

vi.mock('@/platform/distribution/types', () => ({
Expand Down Expand Up @@ -67,6 +68,10 @@ vi.mock('@/platform/settings/settingStore', () => ({
})
}))

vi.mock('../tour/useFirstRunTourController', () => ({
useFirstRunTourController: () => ({ beginTour: mocks.beginTour })
}))

// createSharedComposable caches across calls; each test needs its own instance.
async function freshEntry() {
vi.resetModules()
Expand Down Expand Up @@ -172,6 +177,99 @@ describe('useFirstRunEntry', () => {
}
)

describe('a workflow that arrived by URL', () => {
it.for(['loaded', 'loaded-without-assets'] as const)(
'offers the tour over a shared workflow that %s, which has no template id',
async (sharedStatus) => {
const entry = await freshEntry()

await entry.handleUrlWorkflow('url-intent', undefined, sharedStatus)

expect(
mocks.beginTour,
'a share link is the case no pin can ever cover'
).toHaveBeenCalledWith(undefined)
}
)

it('passes a template id through so its pins beat the heuristic', async () => {
const entry = await freshEntry()

await entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')

expect(mocks.beginTour).toHaveBeenCalledWith('image_z_image_turbo')
})

it('drops the template pins when a share link replaced the graph', async () => {
const entry = await freshEntry()

await entry.handleUrlWorkflow(
'url-intent',
'image_z_image_turbo',
'loaded'
)

expect(
mocks.beginTour,
'pinned ids are graph-local, so validating them against a stranger workflow spotlights whichever node happens to share the id'
).toHaveBeenCalledWith(undefined)
})

it('leaves the completion flag to the startup handler that already settled it', async () => {
const entry = await freshEntry()
await entry.handleStartupOutcome('url-intent')
mocks.setSetting.mockClear()
mocks.beginTour.mockResolvedValue(false)

await entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')

expect(
mocks.setSetting,
'writing it again here would mark onboarding done for a user whose tour never started, and postpone() exists to offer that user the tour again'
).not.toHaveBeenCalled()
})

it.for(['failed', 'cancelled', 'not-present'] as const)(
'offers no tour when nothing the user asked for arrived (%s)',
async (sharedStatus) => {
const entry = await freshEntry()

await entry.handleUrlWorkflow('url-intent', undefined, sharedStatus)

expect(
mocks.beginTour,
'touring a graph the user never asked for is worse than no tour'
).not.toHaveBeenCalled()
}
)

it('leaves a non-candidate alone', async () => {
mocks.isNewUser = false
const entry = await freshEntry()

await entry.handleUrlWorkflow('url-intent', 'image_z_image_turbo')

expect(
mocks.beginTour,
'a returning user opening a share link must not be toured'
).not.toHaveBeenCalled()
})

it.for(['fresh', 'restored'] as const)(
'does not fire on a %s startup',
async (outcome: StartupOutcome) => {
const entry = await freshEntry()

await entry.handleUrlWorkflow(outcome, 'image_z_image_turbo')

expect(
mocks.beginTour,
'only a URL intent has a workflow on the canvas to tour'
).not.toHaveBeenCalled()
}
)
})

it('leaves a completed user alone', async () => {
mocks.settings['Comfy.TutorialCompleted'] = true
const entry = await freshEntry()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ import { useSubscription } from '@/platform/cloud/subscription/composables/useSu
import { isCloud } from '@/platform/distribution/types'
import { useSettingStore } from '@/platform/settings/settingStore'
import type { StartupOutcome } from '@/platform/workflow/persistence/base/draftTypes'
import type { SharedWorkflowUrlLoadStatus } from '@/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader'
import { useNewUserService } from '@/services/useNewUserService'
import { useCommandStore } from '@/stores/commandStore'

import { useFirstRunTourController } from '../tour/useFirstRunTourController'

/**
* Decides what a first-time user sees once startup reports its outcome: the
* Getting Started screen for first-run tour candidates, the template browser
Expand Down Expand Up @@ -57,6 +60,26 @@ export const useFirstRunEntry = createSharedComposable(() => {
await useCommandStore().execute('Comfy.BrowseTemplates')
}

/**
* A share or template link loads its workflow instead of the Getting Started
* screen, so the tour is offered over whatever arrived. The engine declines
* to repeat a tour the user has already seen.
*/
async function handleUrlWorkflow(
outcome: StartupOutcome | undefined,
templateId?: string,
sharedStatus?: SharedWorkflowUrlLoadStatus
) {
if (outcome !== 'url-intent' || !isFirstRunCandidate()) return
const shareLoaded =
sharedStatus === 'loaded' || sharedStatus === 'loaded-without-assets'
if (templateId === undefined && !shareLoaded) return
await useFirstRunTourController().beginTour(
shareLoaded ? undefined : templateId
)
}

// Applied locally before the request, so a failed write is next launch's problem.
async function markTutorialCompleted() {
try {
await settingStore.set('Comfy.TutorialCompleted', true)
Expand All @@ -73,6 +96,7 @@ export const useFirstRunEntry = createSharedComposable(() => {
return {
gettingStartedVisible: readonly(gettingStartedVisible),
handleStartupOutcome,
handleUrlWorkflow,
dismissGettingStarted
}
})
Loading
Loading