Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DESIGN-1854] Light tooltip - Increasing drop-shadow for two increments #321

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

emilmilanov
Copy link
Contributor

@emilmilanov emilmilanov commented Sep 11, 2023

@ryanjwilke
Copy link
Contributor

Task linked: DESIGN-1854 Improve Light Tooltip Shadows

@changeset-bot
Copy link

changeset-bot bot commented Sep 11, 2023

🦋 Changeset detected

Latest commit: f22159e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@cypress-design/react-tooltip Minor
@cypress-design/vue-tooltip Minor
@cypress-design/css Minor
@cypress-design/rollup-plugin-tailwind-keep Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Sep 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
cypress-design ✅ Ready (Inspect) Visit Preview Sep 14, 2023 1:35pm

Copy link
Contributor

@ryanjwilke ryanjwilke left a comment

Choose a reason for hiding this comment

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

Looks solid to me 👍

Copy link
Contributor

@ryanjwilke ryanjwilke left a comment

Choose a reason for hiding this comment

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

@emilmilanov Actually, I just checked how this looks in the preview environment and on dark backgrounds this solution starts to break down a little bit. My suggestion would be to aim for a transparent dark gray shadow (like what we find in Figma). Otherwise, we could consider applying the darken filter on just the drop-shadow, but that would likely be harder to get that to work honestly.

Here's a screenshot:
Screenshot 2023-09-11 at 18 49 18

@elevatebart
Copy link
Contributor

@ryanjwilke on dark backgrounds, the light tooltip stays very visible so we do not have as much a need to see the shadow.
I like what @emilmilanov has done here. My vote goes for "SHIP IT"

@ryanjwilke
Copy link
Contributor

@elevatebart Your comment is more functional in nature. I’m actually referring to something visual in that it would look weird to see a gray shadow on backgrounds that are not white. It may have worked fine for an initial iteration, but we should strive for a solution that brings us closer to the Figma design system.

The drop-shadow really only serves a purpose on lighter backgrounds. All other colors and darker backgrounds, the shadow should essentially be invisible.

@elevatebart
Copy link
Contributor

Thank you for pointing it out.

Don't hesitate to ping me if you are looking for tech solutions.

@emilmilanov
Copy link
Contributor Author

@elevatebart This is ready for a review. I updated after image in the PR description.

It seems like my VS code automatically added few , characters in the vue file.

@cypress
Copy link

cypress bot commented Sep 14, 2023

Passing run #1367 ↗︎

0 136 0 0 Flakiness 0

Details:

Updating tooltip styles
Project: cypress-design Commit: f22159ef61
Status: Passed Duration: 03:45 💡
Started: Sep 14, 2023 1:36 PM Ended: Sep 14, 2023 1:40 PM

Review all test suite changes for PR #321 ↗︎

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

LGTM @ryanjwilke do you like what you see?

@ryanjwilke
Copy link
Contributor

Looks good to me!

@elevatebart elevatebart enabled auto-merge (squash) September 14, 2023 14:56
@elevatebart elevatebart merged commit 1c61ac8 into main Sep 14, 2023
3 checks passed
@elevatebart elevatebart deleted the emil/tooltip-shadow branch September 14, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants