Skip to content

Conversation

Guilhem-lm
Copy link
Contributor

@Guilhem-lm Guilhem-lm commented Oct 8, 2025

Upgrading the UI with new guidelines

Page migrated

  • Worker page
  • Login page
  • Create workspace page
  • User settings
  • Flolder & Groups page
  • Audit logs page
  • Schedules page
  • Http triggers page
  • Variables page
  • Resources page
  • Assets page
  • Home page
  • Runs page
  • Runs detail page
  • Flow Editor
  • SchemaForm
  • Sidebar
  • Flow detail page
  • Script detail page

Components migrated

  • Buttons
  • ToggleButton
  • Tabs
  • TextInput
  • Highlight
  • Dropdown
  • Select
  • Toggle
  • Tooltip
  • Alert
  • DataTable
  • CustomTable

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 8, 2025

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8e60674
Status: ✅  Deploy successful!
Preview URL: https://a7ceab2e.windmill.pages.dev
Branch Preview URL: https://glm-frontend-update-overall.windmill.pages.dev

View logs

@diegoimbert diegoimbert force-pushed the glm/frontend/update-overall-design branch from b569f75 to 4fe569b Compare October 8, 2025 13:35
@Guilhem-lm Guilhem-lm marked this pull request as ready for review October 22, 2025 15:21
@claude
Copy link
Contributor

claude bot commented Oct 22, 2025

Claude finished @Guilhem-lm's task —— View job


Code Review Complete ✅

This is a massive UI redesign effort that successfully introduces a new design system across 100+ files. Here's my comprehensive review:


Major Achievements ✨

1. Design Tokens System Implementation

  • ✅ Well-structured design token system in /frontend/src/lib/assets/tokens/tokens.json
  • ✅ Three theme modes (light, dark, dark-3) with comprehensive color palette
  • ✅ Excellent semantic naming (surface, text, border categories)
  • ✅ Proper integration with Tailwind CSS using CSS custom properties
  • ✅ Clear separation between primitives and semantic tokens

2. Component Migration

  • ✅ Consistent use of Svelte 5 runes throughout ($state, $derived, $bindable, $props)
  • ✅ Proper adoption of snippets instead of slots
  • ✅ Button component has comprehensive variant system (accent, accent-secondary, default, subtle)
  • ✅ Good use of twMerge for dynamic class composition

3. Scope & Coverage

  • ✅ Migrated 20+ pages (Worker, Login, User Settings, Folders, Groups, Audit logs, etc.)
  • ✅ Updated 12+ core components (Button, ToggleButton, Tabs, TextInput, Select, Toggle, etc.)
  • ✅ 8,819 additions, 7,386 deletions - significant refactoring

Issues Found & Recommendations

Critical Issues ⚠️

1. Label Component - Redundant Logic (frontend/src/lib/components/Label.svelte:48)

  • See inline comment for fix

2. Button Dropdown Logic (frontend/src/lib/components/common/button/Button.svelte:163)

  • See inline comment - doesn't handle function case properly

Performance Considerations 🚀

1. Button Component Optimization (frontend/src/lib/components/common/button/Button.svelte:300)

  • getDividerClass could be memoized with $derived
  • Consider: let dividerClass = $derived(getDividerClass(color, variant))

2. Excessive $effect Usage

  • Several components have complex logic in $effect blocks that could be $derived
  • Example: CronInput.svelte has multiple effects that could be simplified

Code Quality Issues 📋

1. Type Safety

  • Some components still mix legacy patterns with Svelte 5
  • Consider removing createEventDispatcher where callback props would suffice

2. Accessibility Concerns

  • Scrollbar color contrast should be tested against WCAG standards
  • Some interactive elements lack proper ARIA attributes
  • Focus management in complex components (modals, dropdowns) should be verified

3. Consistency

  • Mix of old and new Button variants (some using deprecated color prop)
  • Some components still use on: directives instead of direct event handlers

Svelte 5 Best Practices Assessment ✅

Good Practices:

  • ✅ Consistent use of $state, $derived, and $bindable
  • ✅ Proper snippet usage with {@render children?.()}
  • ✅ Good reactive patterns with $effect
  • ✅ Type-safe props interfaces

Areas for Improvement:

  • ⚠️ Some components still use createEventDispatcher (should migrate to callback props)
  • ⚠️ Mix of on:click and onclick event handling
  • ⚠️ Could leverage more $derived for computed values instead of $effect

Architecture & Design Review

Strengths:

  1. Design System Foundation: The token-based approach is excellent and will scale well
  2. Semantic Colors: Using semantic names (surface-primary, text-emphasis) instead of raw colors
  3. Variant System: The new Button variant system (accent, default, subtle) is cleaner than the old color-based approach
  4. Dark Mode Support: Proper support for multiple dark themes

Potential Concerns:

  1. Migration Path: 100 files changed is risky - consider feature flagging or gradual rollout
  2. Bundle Size: Adding new design tokens + keeping old system temporarily increases bundle
  3. Breaking Changes: This will affect any external consumers of these components
  4. Testing Coverage: With changes this extensive, ensure visual regression testing is in place

Security Considerations 🔒

No security issues identified. The changes are primarily UI/UX focused and don't introduce new attack vectors.


Testing Instructions for QA 🧪

To verify the design system changes, the tester should navigate through the Windmill application and check the following:

Visual Consistency Check: Open the Worker page, Login page, User Settings, Folders & Groups management, Audit logs, Schedules, HTTP triggers, Variables, Resources, and the Home page. Verify that all buttons, inputs, dropdowns, and toggles follow the new design guidelines with consistent spacing, colors, and hover states. Check both light and dark modes to ensure the color tokens are applied correctly across all surfaces, text, and borders.

Component Interaction Testing: Test all migrated components by clicking buttons with different variants (accent, default, subtle), interacting with toggle buttons, filling out forms with text inputs, and using dropdown selects. Verify that hover, focus, and active states display correctly, and that disabled states are visually distinct. Pay special attention to the Button component's dropdown functionality.

Flow and Script Editors: Open the Flow Editor and Script Editor to verify the EditorBar, SchemaForm, and sidebar components render correctly. Test creating and editing flows, running scripts, and viewing results to ensure the DisplayResult and FlowStatusViewer components work properly.

Runs and Detail Pages: Navigate to the Runs page and click into individual run details. Check that the Runs detail page, Flow detail page, and Script detail page all display correctly with the new design tokens, especially for status indicators, logs, and result visualization.

Data Tables and Lists: Test the DataTable and CustomTable components by viewing lists in Variables, Resources, Schedules, and other pages. Verify sorting, filtering, and pagination work correctly with the new styling.

Responsive Design: Resize the browser window to test responsive behavior at different breakpoints. The design should adapt gracefully on smaller screens.

Accessibility: Use keyboard navigation (Tab, Enter, Esc) to interact with forms and dialogs. Verify that focus indicators are visible and that all interactive elements are keyboard-accessible. Test with a screen reader if possible to ensure semantic HTML structure is maintained.


Recommendation

APPROVE with minor fixes

This is excellent work that modernizes the UI significantly. The issues found are minor and don't block merging. However, I recommend:

  1. Address the inline comments before merging (especially Label.svelte and Button dropdown logic)
  2. Add visual regression tests to catch future design regressions
  3. Document the new design system (variant usage, token guidelines) for other developers
  4. Monitor performance after deployment - this is a large change
  5. Consider a phased rollout or feature flag given the scope

Great job on this comprehensive redesign! The new design system provides a solid foundation for future UI work. 🎉


@rubenfiszel rubenfiszel merged commit 032f0c1 into main Oct 22, 2025
3 checks passed
@rubenfiszel rubenfiszel deleted the glm/frontend/update-overall-design branch October 22, 2025 15:56
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2025
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