fix(frontend): improve workspace page

[Guilhem-lm]

Key elements

• Add tree structure for better visualisation of the workspace forks
• Add search

Before

After

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	8b5662d
Status:	✅  Deploy successful!
Preview URL:	https://55f77761.windmill.pages.dev
Branch Preview URL:	https://glm-workspace-page-rework.windmill.pages.dev

View logs

[Guilhem-lm]

/ai review PR

[claude]

Claude finished @Guilhem-lm's task in 1m 50s —— View job

---

PR Review Complete ✅

• Read all changed files
• Review new components (WorkspaceCard, WorkspaceIcon, WorkspaceTreeView)
• Review modified files
• Analyze for bugs, performance, and best practices
• Post comprehensive review feedback

---

Overall Assessment

This PR adds a nice tree structure for workspace visualization and search functionality. The implementation is solid overall, with good component decomposition and proper Svelte 5 patterns. Below are some issues and suggestions to consider.

---

🔴 Issues to Address

1. PR Title Typo

The PR title has a typo: "worksace" should be "workspace".

2. Potential XSS Vulnerability (WorkspaceCard.svelte:101)

{#if workspace.marked}
    {@html workspace.marked}
{:else}
    {workspace.name}
{/if}

Using {@html} with the marked property could be a security risk if the HTML isn't properly sanitized. The SearchItems component uses uFuzzy.highlight() which wraps matches in <mark> tags. While this is likely safe since it's generated internally, consider verifying that the highlight function properly escapes the base text to prevent XSS if workspace names contain malicious content.

Fix this →

3. Duplicate cursor-pointer Class (WorkspaceCard.svelte:64-83)

class="px-4 py-2 hover:bg-surface-hover transition-colors cursor-pointer w-full"
...
class:cursor-pointer={!isWorkspaceDisabled(workspace)}

The base class already has cursor-pointer, making the conditional class:cursor-pointer redundant. The cursor-not-allowed is correctly applied but the base cursor-pointer will always be present.

Fix this →

4. Missing async in onkeydown Handler (WorkspaceCard.svelte:75-80)

onkeydown={(e) => {
    if ((e.key === 'Enter' || e.key === ' ') && !isWorkspaceDisabled(workspace)) {
        e.preventDefault()
        onEnterWorkspace(workspace.id)  // Missing await - this is an async function
    }
}}

The onEnterWorkspace returns a Promise but it's not awaited in the keyboard handler (unlike the onclick handler which correctly uses async/await).

Fix this →

---

🟡 Suggestions for Improvement

5. Unused showControls Prop Logic (WorkspaceTreeView.svelte)

The showControls prop is used to conditionally render controls, but in the workspaces page, showControls={false} is passed while the parent component renders its own controls. This design is fine, but the component has duplicated UI that could be simplified.

6. $derived vs $derived.by Inconsistency (WorkspaceMenu.svelte:82-85)

const groupedWorkspaces = $derived(() => {
    if (!$userWorkspaces) return []
    return buildWorkspaceHierarchy($userWorkspaces)
})

This should use $derived.by() for consistency with Svelte 5 patterns when using a function. Using $derived() with a function directly works but $derived.by() is more explicit for computed values that need a function body.

Fix this →

7. SvelteMap Usage in Reactive Context (WorkspaceTreeView.svelte:51)

const childrenMap = new SvelteMap<string, string[]>()

Using SvelteMap inside a $derived.by() block creates a new map on every recomputation. A regular Map would be more appropriate here since the map is only used locally within the computation and doesn't need reactivity.

Fix this →

8. Missing Key for {#each} in Workspaces Page (workspaces/+page.svelte:252-254)

{#each new Array(3) as _}
    <Skeleton layout={[[2], 0.5]} />
{/each}

Missing key for the each block. While this is a static skeleton, adding an index key is a good practice.

9. Tooltip Only Shows When Forked AND Has Parent Name (WorkspaceIcon.svelte:22-24)

{#if isForked && parentName}
    Fork of {parentName}
{/if}

If isForked is true but parentName is undefined, the tooltip renders empty. Consider providing a fallback message like "Fork" or not rendering the tooltip at all.

---

🟢 Positive Observations

1. Good Component Decomposition: Breaking out WorkspaceCard, WorkspaceIcon, and WorkspaceTreeView into separate components improves maintainability.

2. Proper Svelte 5 Usage: Good use of $state, $derived, $bindable, and $props() throughout.

3. Accessibility: The clickable divs include role="button" and tabindex="0" with keyboard handlers for accessibility.

4. New Utility File: workspaceHierarchy.ts is well-documented with JSDoc comments and provides reusable functions.

5. Search with Auto-Expand: The search implementation that auto-expands parent nodes when children match is a nice UX feature.

6. Smooth Animations: Using slide transitions for expand/collapse animations enhances the user experience.

---

Summary

The PR adds valuable functionality with generally good code quality. The main concerns are:

• The potential XSS issue with {@html} (needs verification)
• Minor CSS class conflict
• Missing await on async function call

Once these issues are addressed, this should be good to merge.

[claude]

Claude finished @Guilhem-lm's task in 2m 10s —— View job

---

PR Review Complete ✅

• Read all changed files
• Analyze code quality and best practices
• Check for potential bugs or issues
• Review performance considerations
• Evaluate security implications
• Provide detailed feedback with inline comments
• Write testing instructions

---

Summary

This PR adds a tree structure for workspace visualization with search functionality, improving the UX when managing forked workspaces. The implementation is solid overall with good component decomposition and proper Svelte 5 patterns.

---

Issues Found

I've left inline comments for specific code issues:

Severity	Issue	Location
🟡 Medium	Missing await on async onEnterWorkspace call in keyboard handler	WorkspaceCard.svelte:98
🟢 Low	SvelteMap used inside $derived.by() block (should be regular Map)	WorkspaceTreeView.svelte:87
🟢 Low	Empty tooltip when isForked=true but parentName is undefined	WorkspaceIcon.svelte:22
🟢 Low	Missing key in {#each} loop for skeleton loading	workspaces/+page.svelte:245
ℹ️ Info	{@html} with workspace.marked - verified safe but worth documenting	WorkspaceCard.svelte:119

---

Positive Observations

1. Excellent Component Architecture: Breaking out WorkspaceCard, WorkspaceIcon, and WorkspaceTreeView into separate, focused components follows best practices and improves maintainability.

2. Proper Svelte 5 Patterns: Good use of $state, $derived, $derived.by, $bindable, and $props() throughout the new components.

3. Strong Accessibility Support: The clickable elements properly include role="button", tabindex="0", and comprehensive keyboard handlers (arrow keys, Enter, Space, Escape, Home, End).

4. Well-Documented Utility: workspaceHierarchy.ts includes clear JSDoc comments explaining the purpose and behavior of each function.

5. Smart UX Features:

   • Auto-expanding parent nodes when search matches children
   • Smooth slide transitions for expand/collapse
   • Keyboard navigation with auto-scroll to selected item

6. Reusable Pattern: The WorkspaceMenu.svelte correctly reuses the new buildWorkspaceHierarchy utility function.

---

Testing Instructions

To verify the workspace page changes:

Navigate to the workspaces selection page by logging in and going to /user/workspaces (or clicking "All workspaces" from the sidebar workspace menu). The page should display workspaces in a hierarchical tree structure where forked workspaces appear nested under their parent workspaces with a fork icon. Use the search input at the top to filter workspaces by name - parent nodes should auto-expand when a child workspace matches the search. Click the "Expand"/"Collapse" button to toggle all workspace forks at once. Test keyboard navigation using arrow keys to move between workspaces, Enter/Space to select, and Left/Right arrows to collapse/expand forks. Verify that clicking a workspace card enters that workspace, and that the workspace sidebar menu also shows the hierarchical structure.

---