Skip to content

Conversation

@Guilhem-lm
Copy link
Contributor

Before:
image

After:
image

@Guilhem-lm Guilhem-lm changed the title improve job preview popover layout fix(frontend): improve job preview popover layout Jan 5, 2026
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 6, 2026

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: b614c07
Status: ✅  Deploy successful!
Preview URL: https://8e1bb990.windmill.pages.dev
Branch Preview URL: https://glm-fix-job-preview-popover.windmill.pages.dev

View logs

@Guilhem-lm Guilhem-lm marked this pull request as ready for review January 7, 2026 20:11
@claude
Copy link
Contributor

claude bot commented Jan 7, 2026

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


PR Review: fix(frontend): improve job preview popover layout

  • Analyze the PR changes
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Provide comprehensive feedback

Summary

This PR improves the layout of the job preview popover in the schedules page. The changes reorganize the popover content to display Arguments and Results in a side-by-side layout with a footer showing job metadata (timestamp, memory, duration, labels).

Code Quality Review ✅

Strengths:

  1. Good Svelte 5 patterns - The component correctly uses $state, $derived, and $props() runes
  2. Clean import addition - Added twMerge for class composition, following project patterns
  3. Added customization support - The new class prop enables external styling of the popover

Changes Made:

  1. JobPreview.svelte:

    • Added class prop for external styling customization
    • Reorganized layout: moved Arguments and Results to side-by-side columns (50%/50%)
    • Moved metadata badges from absolute-positioned to flex footer
    • Added section labels ("Arguments", "Result") for better UX
    • Changed shadow-xl to shadow-md (subtle improvement)
    • Used twMerge for clean class composition
  2. schedules/+page.svelte:

    • Added class="p-4" to JobPreview usage for padding

Minor Observations

  1. Line 108: Double space in class string ('absolute z-50 -left-10') - cosmetic only, twMerge handles it
  2. Line 151: The footer div lost absolute bottom-0 right-1 positioning but the new flex-col layout with grow on the content area handles this naturally - this is correct

Overall Assessment

This is a clean, well-implemented UI improvement. The changes follow Svelte 5 best practices, improve the popover's visual hierarchy with proper section labels, and add flexibility through the new class prop. No bugs or issues identified.

Verdict: ✅ Looks good to merge


Testing Instructions

Navigate to the Schedules page in the Windmill app. Hover over any of the colored job status bars (the small vertical colored bars showing recent job executions for each schedule) to see the job preview popover. The popover should display with Arguments on the left side and Results on the right side, with job metadata (timestamp, memory, duration) in a footer at the bottom.

@rubenfiszel rubenfiszel merged commit 61d357f into main Jan 8, 2026
13 checks passed
@rubenfiszel rubenfiszel deleted the glm/fix-job-preview-popover branch January 8, 2026 06:29
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants