fix(frontend): improve centered page overflow

[Guilhem-lm]

• Prevent page shift when centered page overflow
• Update approval page to brand guidelines

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	85896ec
Status:	✅  Deploy successful!
Preview URL:	https://4dd2e0ad.windmill.pages.dev
Branch Preview URL:	https://glm-improve-centered-page-pl.windmill.pages.dev

View logs

[Guilhem-lm]

/ai review PR

[claude]

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

---

PR Review: fix(frontend): improve centered page overflow

• Read changed files
• Analyze CenteredModal.svelte changes
• Review workspaces page updates
• Review approve page updates
• Check other modified files
• Provide comprehensive review feedback

---

Summary

This PR makes two main improvements:

1. Prevents page shift when centered page overflows - by adding scrollbar-gutter: stable both-edges and switching to overflow-auto
2. Updates the approval page styling to brand guidelines - improves typography, adds proper card styling, and enhances visual hierarchy

---

✅ What looks good

1. CenteredModal.svelte improvements:

   • Good migration from export let to $props() with proper TypeScript interface - follows Svelte 5 best practices
   • scrollbar-gutter: stable both-edges is an excellent solution for preventing layout shift on scrollbar appearance
   • The centerVertically prop provides flexibility for pages with variable content height
   • Using h-screen + overflow-auto is a solid approach for containing the modal in the viewport

2. Approval page updates:

   • Typography improvements with consistent text-sm font-semibold text-emphasis for headers
   • The bg-surface-tertiary shadow-md card treatment for the action area provides good visual hierarchy
   • Using Alert components for status messages (completed, already resumed) is cleaner than raw text
   • Button size change from md to lg improves touch targets

3. Workspaces page:

   • Adding centerVertically={false} makes sense for content-heavy pages
   • Good restructuring by moving nonForkInvites calculation to a @const block

---

⚠️ Issues to address

1. Debug statement left in code (approve page, line 158)

$inspect('dbg schema', schema)

This should be removed before merging. Fix this →

2. Empty schema check could be more robust (approve page, line 248)

{#if schema && Object.keys(schema).length > 0 && !completed}

The previous check was just {#if schema && !completed}. If this stricter check is intentional (to avoid showing empty schema forms), that's fine - but consider if this could cause issues with schemas that have nested properties but no top-level keys.

---

💡 Suggestions (optional)

1. Consider adding a transition for smoother scroll appearance:
   While scrollbar-gutter prevents shift, you could also consider overflow: overlay (with fallback) for browsers that support it, which provides an even smoother scrollbar experience.

2. WindmillIcon size parameter:
   The change from height="100px" width="100px" to size={centerVertically ? 64 : 48} looks good, but verify the WindmillIcon component accepts a size prop correctly (vs separate height/width).

3. Removing bottom margin from content card:
   The removal of mb-6 md:mb-20 from the content card might affect spacing in some edge cases. The new pb-8 on the container should compensate, but worth double-checking on actual pages.

---

Overall assessment

LGTM with one required fix 🟡

The PR is well-executed overall with good attention to brand guidelines and UX improvements. The only blocking issue is the debug $inspect statement that must be removed before merging. Once that's addressed, this is ready to merge.

[claude]

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

---

PR Review: fix(frontend): improve centered page overflow

• Read and analyze changed files
• Review CenteredModal.svelte changes
• Review workspaces page updates
• Review approve page updates
• Check other modified files
• Provide comprehensive review feedback with inline comments

---

Summary

This PR makes two main improvements:

1. Prevents page shift when centered page overflows - by adding scrollbar-gutter: stable both-edges and switching to overflow-auto
2. Updates the approval page styling to brand guidelines - improves typography, adds proper card styling, and enhances visual hierarchy

---

✅ What looks good

1. CenteredModal.svelte improvements:

• Excellent migration from export let to $props() with proper TypeScript interface - follows Svelte 5 best practices
• scrollbar-gutter: stable both-edges is the correct CSS solution for preventing layout shift when scrollbar appears/disappears
• The new centerVertically prop provides flexibility for pages with variable content height
• Using h-screen + overflow-auto properly contains the modal in the viewport
• Good use of {@render children?.()} snippet pattern for content projection

2. Workspaces page restructuring:

• Smart use of centerVertically={false} for content-heavy pages that may exceed viewport height
• Good restructuring by moving nonForkInvites calculation to a {@const} block inside the template (line 181)
• Consistent header styling with text-sm font-semibold text-emphasis

3. Approval page updates:

• The bg-surface-tertiary shadow-md card treatment for the action area provides good visual hierarchy (line 225)
• Using Alert components for status messages (completed, already resumed) is cleaner than raw text
• Button size change from md to lg improves touch targets for the important approve/deny actions
• Headers are consistently styled with text-sm font-semibold text-emphasis

4. Other files:

• CreateWorkspace.svelte (line 346), instance_settings (line 10), and cli (line 28) correctly pass centerVertically={false} where appropriate for their content

---

⚠️ Issues to address

1. Debug statement was left but now removed ✅
The previous review mentioned a $inspect('dbg schema', schema) debug statement. I confirmed this has been removed in commit 85896ec.

2. Stricter schema check on approval page (line 246)

{#if schema && Object.keys(schema).length > 0 && !completed}

This is a stricter check than before. While likely intentional to avoid showing empty schema forms, consider if this could cause issues with schemas that have nested properties but no top-level keys. If this is the intended behavior, a brief comment would help future maintainers understand the logic.

---

💡 Suggestions (optional)

1. Consider scrollbar-gutter browser support:
scrollbar-gutter has good modern browser support (Chrome 94+, Firefox 97+, Safari 17+), but may not work on older browsers. The layout won't break - it will just have the slight shift that existed before. Worth keeping in mind for compatibility requirements.

2. WindmillIcon size prop usage:
The change from height="100px" width="100px" to size={centerVertically ? 64 : 48} (line 37) looks good. I verified the WindmillIcon component accepts a size prop.

---

Overall assessment

LGTM 🟢

The PR is well-executed with good attention to brand guidelines and UX improvements. The debug statement has been removed, and all changes follow Svelte 5 best practices with proper use of $props(), $state(), $derived(), and $effect(). The code is clean and maintainable.

---

Testing Instructions

To verify the changes, navigate to any of the centered modal pages:

1. Workspaces page (/user/workspaces): Log in and observe the workspace selection modal. If you have many workspaces that cause vertical overflow, verify there's no horizontal page shift when the scrollbar appears. The content should now be top-aligned rather than vertically centered.

2. Approval page (requires a suspended flow with approval step): Start a flow that has an approval step, then visit the approval link. Verify the improved styling - the approvers section, flow arguments, and the approve/deny action area should have a clean card design with proper visual hierarchy.

3. Create workspace page (/user/create_workspace): Check that the form is properly displayed without vertical centering (content starts near the top).

4. CLI authorization page (/user/cli?port=8000): Navigate here to verify the login authorization modal displays correctly.

The key visual improvement to check is that on any page with overflow, the content doesn't shift horizontally when the scrollbar appears/disappears.
| Preview URL