fix surface hover bug

[diegoimbert]

No description provided.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	b094f95
Status:	✅  Deploy successful!
Preview URL:	https://d7d6c6c8.windmill.pages.dev
Branch Preview URL:	https://di-nit-hover-opacity-bug-fix.windmill.pages.dev

View logs

[claude]

Claude finished @diegoimbert's task —— View job

---

Code Review: fix surface hover bug

Todo List:

• Examine the PR changes
• Review each modified file
• Provide comprehensive feedback
• Include testing instructions

---

Review Summary

This PR addresses opacity issues with hover states on surface elements by standardizing hover background colors and adjusting text styling in dropdown components. The changes are minimal and focused, affecting three Svelte components.

Changes Analysis

1. AssetsDropdownButton.svelte (Line 121)

Change: Removed hover:bg-surface-hover/25 class from list items

Analysis:

• ✅ Good: Removes redundant hover styling that was likely causing visual inconsistency
• ✅ The component already has onmouseenter and onmouseleave handlers, suggesting hover behavior might be controlled programmatically
• ⚠️ Question: With hover styling removed, is the hover state now handled entirely through the onHoverLi callback? If so, this should be verified to ensure users still get visual feedback

2. Badge.svelte (Line 106)

Change: Changed hover:bg-surface-hover/50 to hover:bg-surface-hover for transparent badges

Analysis:

• ✅ Excellent: Standardizes opacity usage across the codebase
• ✅ Improves visual consistency - the /50 opacity modifier was likely too subtle or inconsistent with other hover states
• ✅ Maintains proper semantic color naming using design tokens
• ✅ Change is limited to the transparent badge variant, preserving other badge styles

3. SelectDropdown.svelte (Lines 194, 196, 212)

Changes:

• Lines 194, 196: Changed bg-surface-hover/60 to bg-surface-secondary for selected/focused states
• Line 212: Changed subtitle from text-xs text-primary to text-2xs text-secondary

Analysis:

• ✅ Great improvement: Uses bg-surface-secondary instead of opacity-modified bg-surface-hover for selected/focused items
• ✅ This creates better semantic separation: surface-hover for hover states vs surface-secondary for selected states
• ✅ The subtitle styling change (text-xs → text-2xs, text-primary → text-secondary) improves visual hierarchy
• ✅ Smaller, lighter text for subtitles provides better visual differentiation from main labels
• ✅ Maintains accessibility by keeping hover state distinct (hover:bg-surface-hover on line 197)

Code Quality Assessment

Strengths

• Consistency: Changes align well with Tailwind and design system patterns
• Semantic correctness: Uses appropriate color tokens for different UI states
• Minimal scope: Focused changes reduce risk of unintended side effects
• No breaking changes: All changes are purely visual/styling

Considerations

1. Design System Alignment: ⚠️

   • Verify that bg-surface-secondary is defined in your design system and renders appropriately in both light and dark modes
   • Ensure text-2xs is a defined utility class

2. Hover Behavior in AssetsDropdownButton: ⚠️

   • The removal of hover styling should be tested to ensure users still get adequate feedback
   • Consider whether CSS hover or the programmatic onHoverLi callback provides the visual feedback

3. Cross-browser Testing: 💡

   • Test in different browsers to ensure opacity changes render consistently
   • Verify dark mode appearance

Svelte 5 Best Practices Compliance

✅ The code follows Svelte 5 patterns:

• Uses $state, $derived, $props runes appropriately
• Uses onclick instead of on:click directive
• Proper use of snippets with {@render}
• Scoped styling with Tailwind

Performance & Security

• ✅ Performance: No performance impact - purely CSS changes
• ✅ Security: No security implications
• ✅ Accessibility: Color contrast should be verified for text-secondary in subtitle text

Testing Instructions

To verify these hover bug fixes, navigate through the Windmill app interface and test the following interactive elements:

Assets Dropdown: Open any view that displays asset selection dropdowns (typically found in script editors, flow builders, or resource managers). Hover over items in the asset list and verify that hover states are visually clear and consistent without excessive opacity effects.

Badge Components: Look for transparent badges throughout the UI (commonly used for tags, filters, or status indicators in script lists, app builders, or workspace settings). Hover over these badges and confirm the hover effect is visible and appropriately opaque, providing clear visual feedback.

Select Dropdowns: Use any select dropdown components found in forms, settings panels, or configuration dialogs. Navigate through the dropdown options using both mouse hover and keyboard arrow keys, verifying that the selected/focused state (blue-tinted background) is distinct from the hover state (lighter background). Also check that subtitle text in dropdown options appears smaller and lighter than the main label text.

Test both light and dark modes to ensure the surface color changes work correctly across themes.

---

Recommendation

✅ APPROVE - This is a well-targeted bug fix that improves visual consistency. The changes are low-risk and address opacity issues systematically across multiple components.

Suggested next steps:

1. Verify that bg-surface-secondary and text-2xs are properly defined in your Tailwind configuration
2. Test in both light and dark modes
3. Confirm hover feedback is still adequate in AssetsDropdownButton after removing the explicit hover class