feat: new live onboarding for flows

[tristantr]

No description provided.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	70a0717
Status:	✅  Deploy successful!
Preview URL:	https://05b81e11.windmill.pages.dev
Branch Preview URL:	https://tl-workspace-onboarding.windmill.pages.dev

View logs

[claude]

Claude finished @tristantr's task —— View job

---

Code Review Complete ✓

I've completed a comprehensive review of PR #7194 which adds a new live onboarding feature for flows. Here's my assessment:

---

🚨 Critical Issues

1. Svelte 4 Syntax in New File (Must Fix)

WorkspaceOnboardingTutorial.svelte uses Svelte 4 syntax (export let) instead of required Svelte 5 runes. This violates the project's coding standards which mandate Svelte 5 for all new files.

Required changes:

• Convert export let to $props()
• Convert let tutorial to use $state()
• Replace on:error and on:skipAll event dispatchers with callback props
• Use the Svelte migration tool to automatically convert this file

---

⚠️ High Priority Issues

2. Verbose Helper Functions

Lines 32-64 in WorkspaceOnboardingTutorial.svelte contain unnecessary helper functions that just wrap document.querySelector() with logging. The code still validates the elements exist later, making these wrappers redundant.

3. Large Bundle Size

FlowBuilderTutorialWorkspaceOnboardingContinue.svelte embeds 670+ lines including a full flow definition with hardcoded scripts (lines 148-220). This significantly bloats the bundle. Consider:

• Moving flow templates to separate JSON files
• Lazy loading tutorial content
• Code splitting to only load when tutorial is triggered

4. Race Condition & Poor UX

The tutorial blocks the "Next" button without feedback when step4Complete is false (lines 438-443). If the code typing animation fails or stalls, users are stuck with no indication why. Add user feedback (toast notification) when blocking.

---

🔍 Medium Priority Issues

5. Excessive DOM Manipulation

The tutorial creates fake cursors, custom overlays, and performs heavy DOM manipulation with hardcoded selectors like 'button[title="flow_input.celsius"]'. This is:

• Fragile: Breaks if UI changes
• Performance concern: Frequent DOM element creation/removal
• Maintenance burden: Hardcoded selectors and timing dependencies

Recommendations:

• Use data attributes for tutorial targets
• Prefer CSS animations over JS manipulation
• Add error handling when elements aren't found

6. Tutorial Timing Issues

Both auto-start locations use fixed delays (500ms) which may be insufficient for slow networks or heavy pages. Consider polling for required DOM elements before starting.

7. Potential XSS Risk

Line 88 in FlowBuilderTutorialWorkspaceOnboardingContinue.svelte injects raw HTML into popover descriptions. While currently using static content, this pattern could become a security issue if dynamic content is added later.

---

✅ Positive Observations

1. Good Svelte 5 usage in guides/+page.svelte - proper use of $state() and onclick
2. Comprehensive tutorial flow - The temperature converter example is well-designed pedagogically
3. Tutorial infrastructure - Good integration with existing tutorial system
4. User experience focus - Animations and guided interactions enhance learning

---

🔒 Security Assessment

• Low Risk: No direct security vulnerabilities found
• Watch: HTML injection in popover descriptions (line 88) - ensure no dynamic content is ever added there
• Good: No exposure of sensitive data, proper localStorage cleanup

---

🎯 Performance Considerations

• Bundle size: Large embedded flow data increases initial load
• DOM operations: Heavy manipulation could cause jank on slower devices
• Timing dependencies: Many setTimeout calls could accumulate

---

📋 Testing Instructions

To test the new workspace onboarding feature:

First-time user experience: Navigate to the workspace home page after logging in for the first time (or clear tutorial progress in localStorage). The onboarding tutorial should automatically start after 500ms, presenting a welcome message and highlighting the "Create Script", "Create Flow", and "Create App" buttons with explanatory popovers showing images of each feature. When you proceed through the introduction, you'll be automatically redirected to the flow builder where an interactive tutorial will guide you through building a temperature converter flow step-by-step, including watching code being typed automatically, connecting inputs, and testing the complete flow.

Manual trigger: On the workspace home page, click the "Tutorial" button in the page header to manually start the onboarding tutorial at any time. You can also access it from the new Guides page (navigate to /guides and click "Create a simple flow" under the Quickstart tab), which will launch the flow builder portion of the tutorial directly with a fresh state, bypassing the workspace introduction.

URL-based activation: The tutorial can be triggered via URL parameter by navigating to /flows/add?tutorial=workspace-onboarding-continue&nodraft=true, which skips the home page introduction and jumps directly to the interactive flow building tutorial in the flow editor.

---

📝 Recommendations Summary

Must fix before merge:

1. Convert WorkspaceOnboardingTutorial.svelte to Svelte 5 syntax

Should fix:
2. Remove verbose helper functions
3. Add user feedback for blocked tutorial progression
4. Extract embedded flow data to reduce bundle size

Consider for follow-up:
5. Use data attributes instead of fragile selectors
6. Add robust element-ready checking before tutorial start
7. Document HTML injection security considerations

---

[claude]

Critical: Use Svelte 5 Runes Instead of export let

This file uses Svelte 4 syntax (export let) instead of the required Svelte 5 runes. According to the project's Svelte 5 best practices:

Suggested change

	export let index: number = 8
	let { name = 'workspace-onboarding', index = 8 } = $props()

Additional issues to fix:

• Line 10: Change let tutorial: Tutorial | undefined = undefined to let tutorial: Tutorial | undefined = $state(undefined)
• Line 24: Remove the bind:this={tutorial} binding on the Tutorial component and use a different pattern if needed
• Lines 28-29: Replace event dispatchers on:error and on:skipAll with callback props

You can use the Svelte 4 to Svelte 5 migration script from VS Code/Cursor with the Svelte extension to automatically convert this file.

[claude]

Verbose Helper Functions

As noted in the review, these helper functions are unnecessarily verbose since you still check if the button exists later. Consider simplifying to:

Suggested change

	// Helper function to find the create script button
	const findScriptButton = (): HTMLElement | null => {
	const button = document.querySelector('#create-script-button') as HTMLElement | null
	if (button) {
	console.log('Found script button:', button)
	} else {
	console.error('Could not find script button')
	}
	return button
	}
	// Helper function to find the create flow button
	const findFlowButton = (): HTMLElement | null => {
	const button = document.querySelector('#create-flow-button') as HTMLElement | null
	if (button) {
	console.log('Found flow button:', button)
	} else {
	console.error('Could not find flow button')
	}
	return button
	}
	// Helper function to find the create app button
	const findAppButton = (): HTMLElement | null => {
	const button = document.querySelector('#create-app-button') as HTMLElement | null
	if (button) {
	console.log('Found app button:', button)
	} else {
	console.error('Could not find app button')
	}
	return button
	}
	const steps: DriveStep[] = [
	{
	popover: {
	title: 'Welcome to your Windmill workspace! 🎉',
	description:
	"Let's take a quick tour! In this tutorial, we'll create a simple flow so you",
	onNextClick: () => {
	// Wait a bit to ensure the page is fully rendered before moving to next step
	setTimeout(() => {
	const button = document.querySelector('#create-script-button') as HTMLElement | null
	if (button) {
	driver.moveNext()
	} else {
	alert('Could not find the Create Script button. Please make sure you are on the home page.')
	}
	}, 100)
	}
	}
	},

Then use this pattern directly in each step instead of calling the helper functions.

[claude]

Use on: prefix for event handlers in Svelte 5

While using on:error and on:skipAll works in compatibility mode, Svelte 5 best practices recommend using callback props instead of event dispatchers. Consider changing the Tutorial component to accept callback props like onerror and onskipAll.

However, if you're keeping the on: pattern for backwards compatibility with existing code, this is acceptable.

[claude]

Potential Performance Issue: Large Embedded Flow Data

This file contains a hardcoded flow structure with all the code content embedded (lines 148-220). This significantly increases the bundle size for this component. Consider:

1. Lazy loading: Load this data dynamically when the tutorial starts
2. External file: Move the flow definition to a separate JSON file that can be imported
3. Code splitting: Ensure this large component is only loaded when the tutorial is actually triggered

Example:

// Create a separate file: flowTemplates.ts
export const temperatureConverterFlow = { ... }

// Then import it here
import { temperatureConverterFlow } from './flowTemplates'
const flowJson: Flow = temperatureConverterFlow

[claude]

Excessive DOM Manipulation and Animations

This tutorial has extensive DOM manipulation with fake cursors, custom overlays, and complex animations. While this creates a nice user experience, there are several concerns:

1. Performance: Creating/removing DOM elements frequently (lines 473-587) can cause layout thrashing
2. Maintainability: Hardcoded selectors like 'button[title="flow_input.celsius"]' are fragile and will break if the UI changes
3. Timing dependencies: Heavy reliance on setTimeout and wait() calls makes this brittle

Recommendations:

• Consider using CSS animations instead of JS-based transitions where possible
• Use more robust selectors or data attributes (e.g., data-tutorial-target) instead of hardcoded titles
• Add error handling for when elements aren't found
• Consider if some of these animations could be simplified without losing the educational value

[claude]

Race Condition: Blocking on step4Complete flag

The onNextClick handler blocks if step4Complete is false, but there's no user feedback or timeout. If the code typing animation fails or takes longer than expected, the user will be stuck without knowing why the "Next" button doesn't work.

Recommendation:

onNextClick: () => {
    if (!step4Complete) {
        // Show a toast or visual feedback
        sendUserToast('Please wait for the code to finish typing...', false)
        return
    }
    // ... rest of the code
}

[claude]

Potential XSS Risk: Unescaped HTML in Description

HTML is being injected directly into the popover description. While this appears to be static content, it's a potential security risk if any dynamic content is ever added here.

Recommendation:
Use Svelte's snippet feature or a safer approach:

{#snippet popoverDescription()}
    <img src="/languages.png" style="width: 100%; max-width: 400px; margin-bottom: 12px; border-radius: 8px;" />
    <p>Click to create your first script!</p>
{/snippet}

Or at minimum, add a comment documenting that this HTML must never contain user input.

[claude]

Tutorial Auto-Start on Page Load

The tutorial automatically starts 500ms after page load if the user hasn't completed or ignored it. While this is intentional, consider:

1. User experience: Some users might find auto-starting tutorials intrusive, especially on subsequent visits
2. Consider a preference: Add a user preference to disable auto-start
3. Analytics: Track if users are dismissing the tutorial immediately to gauge if the auto-start is helpful

The 500ms delay is good to ensure the page is loaded, but you might want to also check if critical elements exist before starting.

[claude]

Tutorial Trigger Timing

The tutorial trigger logic has two paths:

1. URL parameter: 500ms delay
2. Fresh flow creation: Uses tick() which is immediate

Potential Issue: The 500ms delay might not be sufficient if there are slow network requests or heavy rendering. Consider:

// Use a more robust approach
await tick()
// Wait for critical elements to be ready
let attempts = 0
while (attempts < 20 && !document.querySelector('#flow-editor-virtual-Input')) {
    await new Promise(resolve => setTimeout(resolve, 100))
    attempts++
}
flowBuilder?.triggerTutorial()

This ensures the tutorial only starts when the page is actually ready.

[claude]

Good: Proper Svelte 5 Usage

This file correctly uses Svelte 5 syntax with $state() and the onclick event handler (not on:click). This is a good example of following the project's Svelte 5 best practices.

[claude]

Consider: Direct navigation might skip cleanup

Using window.location.href forces a full page reload. While this ensures a fresh state, consider if goto() from $app/navigation would be more appropriate to maintain SPA behavior and avoid unnecessary full page reloads.

However, if the full reload is intentional to ensure localStorage is cleared and a completely fresh tutorial start, this is fine.