-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[TreeView] Fix drag and drop color usage #15042
[TreeView] Fix drag and drop color usage #15042
Conversation
Deploy preview: https://deploy-preview-15042--material-ui-x.netlify.app/ |
@@ -25,51 +25,30 @@ const TreeItem2DragAndDropOverlayRoot = styled('div', { | |||
marginLeft: 'calc(var(--TreeView-indentMultiplier) * var(--TreeView-itemDepth))', | |||
borderRadius: theme.shape.borderRadius, | |||
backgroundColor: theme.vars | |||
? `rgba(${theme.vars.palette.primary.dark} / ${0.15})` | |||
: alpha(theme.palette.primary.dark, 0.15), | |||
? `rgba(${theme.vars.palette.primary.darkChannel} / ${theme.vars.palette.action.focusOpacity})` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only <color>Channel
variables can be used in rgba
as they provide the separate rgb values (i.e. 255 255 255).
Suggesting usage of a variable for opacity.
The default value is very close (0.12):
https://github.com/mui/material-ui/blob/85422cf137f1fc4ac9803f0af3e2b093ea21bb06/packages/mui-material/src/styles/createPalette.js#L47
? `rgba(${theme.vars.palette.grey[100]} / ${0.6})` | ||
: alpha(theme.palette.grey[100], 0.6), | ||
}), | ||
borderTop: `1px solid ${(theme.vars || theme).palette.action.active}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting a simplification on these cases.
grey
colors do not have a channel
to be used in the rgba
function. 🤷
action.active
changes depending on the theme and looks quite close to the current colors.
I'm letting @noraleonte check the diff since the did the original version 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the improvement 💙
Fixes #15017.
This is a suggestion/simplification.
Feel free to suggest better colors/option that would work with CSS Variables as well. 👍