Skip to content

Conversation

viva-jinyi
Copy link
Collaborator

@viva-jinyi viva-jinyi commented Sep 6, 2025

Summary

  • Fixed the close button visibility issue in the manager dialog warning banner
  • Replaced custom button implementation with reusable IconButton component
  • Simplified banner styling by removing unnecessary border styling

Changes

  • Replaced manual button element with IconButton component for better consistency
  • Updated button positioning to use top-0 right-0 for proper alignment
  • Changed background opacity syntax from bg-opacity-20 to modern Tailwind bg-yellow-500/20
  • Removed yellow border that was making the UI too busy
  • Ensured close icon is visible with white text color

Test Plan

  • Open the manager dialog
  • Verify warning banner appears when there are conflicts
  • Confirm close button is visible and properly positioned
  • Test clicking close button dismisses the banner
  • Check dark mode compatibility

Fixes visibility issue where close button was hard to see against warning banner background.

🤖 Generated with Claude Code

┆Issue is synchronized with this Notion page by Unito

@viva-jinyi viva-jinyi requested a review from a team as a code owner September 6, 2025 14:17
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 6, 2025
Copy link

github-actions bot commented Sep 6, 2025

🎭 Playwright Test Results

All tests passed across all browsers!

⏰ Completed at: 09/07/2025, 04:48:52 AM UTC

📊 Test Reports by Browser


🎉 Your tests are passing across all browsers!

@viva-jinyi viva-jinyi self-assigned this Sep 6, 2025
@viva-jinyi viva-jinyi added area:manager 1.26 backport Backporting a PR onto a release candidate needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch and removed backport Backporting a PR onto a release candidate labels Sep 6, 2025
@christian-byrne
Copy link
Contributor

What does it look like now?

@viva-jinyi
Copy link
Collaborator Author

@christian-byrne

Before

스크린샷 2025-09-07 오전 9 22 18 스크린샷 2025-09-07 오전 9 22 29

After

스크린샷 2025-09-07 오후 1 30 02 스크린샷 2025-09-07 오후 1 29 37

It looks like while resolving the merge conflict, the original styles were overridden with different ones. I’ve corrected it so that it now matches the original styling.

@viva-jinyi viva-jinyi force-pushed the manager/close-icon-invisible branch from 9490e9a to 08d6afc Compare September 7, 2025 04:32
Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@christian-byrne christian-byrne merged commit 5588292 into main Sep 7, 2025
19 checks passed
@christian-byrne christian-byrne deleted the manager/close-icon-invisible branch September 7, 2025 04:49
github-actions bot pushed a commit that referenced this pull request Sep 7, 2025
…5397)

* feature: manager banner style fix

* fix: light-theme color

* fix: icon color modified for dark theme
@comfy-pr-bot
Copy link
Member

@viva-jinyi Successfully backported to #5414

christian-byrne pushed a commit that referenced this pull request Sep 7, 2025
…5397) (#5414)

* feature: manager banner style fix

* fix: light-theme color

* fix: icon color modified for dark theme

Co-authored-by: Jin Yi <[email protected]>
This was referenced Sep 7, 2025
snomiao pushed a commit that referenced this pull request Sep 12, 2025
…5397)

* feature: manager banner style fix

* fix: light-theme color

* fix: icon color modified for dark theme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.26 area:manager needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants