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

[Tooltip] Fix tooltip arrow css var background #33753

Merged
merged 2 commits into from Aug 3, 2022

Conversation

TimoWilhelm
Copy link
Contributor

This PR fixes the Tooltip arrow background color, to use the same color as the tooltip if CSS variables are used.

@mui-bot
Copy link

mui-bot commented Aug 2, 2022

Details of bundle changes

Generated by 🚫 dangerJS against 5f878d4

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Thanks for the fix!

Even though, the Tooltip background is alpha(palette.grey[700], 0.92)) but I'd say that your fix makes more sense.

@TimoWilhelm
Copy link
Contributor Author

TimoWilhelm commented Aug 2, 2022

Even though, the Tooltip background is alpha(palette.grey[700], 0.92)) but I'd say that your fix makes more sense.

For some reason, the arrow was displaying in a different color in my case.

image
image

However, when using theme.vars.palette.Tooltip.bg it looks fine.

@propG
Copy link

propG commented Aug 2, 2022

Trying to get the back ground

@siriwatknp
Copy link
Member

Even though, the Tooltip background is alpha(palette.grey[700], 0.92)) but I'd say that your fix makes more sense.

For some reason, the arrow was displaying in a different color in my case.

image image

However, when using theme.vars.palette.Tooltip.bg it looks fine.

Sorry, I don't quite get it. Can you share the minimal reproduction via a CodeSandbox?

@TimoWilhelm
Copy link
Contributor Author

@siriwatknp
Copy link
Member

@siriwatknp https://codesandbox.io/s/mui-issue-33753-6ijj4z?file=/src/App.tsx

Ah, I see. This is a bug because there is no darkChannel in grey.

// TooltipArrow
color: theme.vars
    ? `rgba(${theme.vars.palette.grey.darkChannel} / 0.9)`
    : alpha(theme.palette.grey[700], 0.9),

Screen Shot 2565-08-03 at 08 42 12

@siriwatknp siriwatknp added bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Aug 3, 2022
@siriwatknp siriwatknp merged commit c6ce976 into mui:master Aug 3, 2022
@siriwatknp
Copy link
Member

@TimoWilhelm Are you using CSS variables in your project? Would love to hear your feedback about the overall experience.

@TimoWilhelm
Copy link
Contributor Author

Thanks for the quick PR review @siriwatknp.

Yep, I used CSS variables since it became available and my experience has been really positive so far. I strongly believe this is the future of the library.

My favorite features are avoiding the color-scheme flash with SSR and preventing any hydration mismatch errors.

I haven't found a nice way yet to access the CSS variables for styling beyond components (e.g. global scrollbar track/thumb) and I wish the CSP hash for the getInitColorSchemeScript was a bit easier to handle (#27651 (comment)) but overall I'm really happy with the current state of the CSS variable support!

@siriwatknp
Copy link
Member

Thanks for the quick PR review @siriwatknp.

Yep, I used CSS variables since it became available and my experience has been really positive so far. I strongly believe this is the future of the library.

My favorite features are avoiding the color-scheme flash with SSR and preventing any hydration mismatch errors.

I haven't found a nice way yet to access the CSS variables for styling beyond components (e.g. global scrollbar track/thumb) and I wish the CSP hash for the getInitColorSchemeScript was a bit easier to handle (#27651 (comment)) but overall I'm really happy with the current state of the CSS variable support!

Thanks for sharing!

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants