Skip to content

fix: DH-18562: Deephaven picker hover blends with non-hovered styling in dark mode#2433

Merged
gzh2003 merged 1 commit intodeephaven:mainfrom
gzh2003:DH-18562-deephaven-ui-picker-hover-blends-in-with-non-hovered-styling-in-dark-mode
May 6, 2025
Merged

fix: DH-18562: Deephaven picker hover blends with non-hovered styling in dark mode#2433
gzh2003 merged 1 commit intodeephaven:mainfrom
gzh2003:DH-18562-deephaven-ui-picker-hover-blends-in-with-non-hovered-styling-in-dark-mode

Conversation

@gzh2003
Copy link
Copy Markdown
Contributor

@gzh2003 gzh2003 commented May 6, 2025

DH-18562: Fixes issue with picker styling when using dark mode. Previously the background did not serve as a good indicator when hovering over items.

I checked in with @dsmmcken to confirm the styling changes. I also tested the picker component in both the style guide and theme selector under settings.

Hovered Styling (Before)
Before Screenshot

Hovered Styling (After)
After Screenshot

@gzh2003 gzh2003 requested a review from mofojed May 6, 2025 17:53
@gzh2003 gzh2003 self-assigned this May 6, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2025

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.06%. Comparing base (6d65594) to head (0174eb0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2433   +/-   ##
=======================================
  Coverage   47.06%   47.06%           
=======================================
  Files         714      714           
  Lines       39468    39468           
  Branches    10063    10063           
=======================================
  Hits        18576    18576           
  Misses      20881    20881           
  Partials       11       11           
Flag Coverage Δ
unit 47.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gzh2003 gzh2003 force-pushed the DH-18562-deephaven-ui-picker-hover-blends-in-with-non-hovered-styling-in-dark-mode branch from 6feff1d to 0174eb0 Compare May 6, 2025 18:00
@gzh2003
Copy link
Copy Markdown
Contributor Author

gzh2003 commented May 6, 2025

I have read the CLA Document and I hereby sign the CLA

deephaven-internal added a commit to deephaven/cla that referenced this pull request May 6, 2025
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

I assume you checked with @dsmmcken about which colour variable to use.

In the PR description, I encourage including details about how you tested a particular ticket (e.g. "Checked using the theme selector under settings") so it's clear what has been tested (for both the current reviewer, and yourself or any other developer who may come across this ticket in the future); and for a ticket like this which is a small visual change, a screenshot of before/after is also good.

Looks great, great work!

@gzh2003 gzh2003 merged commit e6ed762 into deephaven:main May 6, 2025
14 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 6, 2025
@gzh2003 gzh2003 deleted the DH-18562-deephaven-ui-picker-hover-blends-in-with-non-hovered-styling-in-dark-mode branch May 6, 2025 20:22
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.

2 participants