[AppBar] Add color transparent support #19393
[AppBar] Add color transparent support #19393oliviertassinari merged 3 commits intomasterfrom unknown repository
Conversation
|
A fix for #19391. |
ghost
requested review from
Details of bundle changes.Comparing: 39c8170...e269421
|
oliviertassinari
changed the title
AppBar color='inherit'
oliviertassinari
left a comment
oliviertassinari
left a comment
There was a problem hiding this comment.
I don't think that we can accept the changes.
The use case is about keeping the background with a fixed positioning while leveraging color inheritance
oliviertassinari
added
the
not planned
I don't understand this. Can you please explain what's the intended strategy for |
|
@lexskir I have looked at an older project I worked on and noticed we have used the color="inherit" prop only so we would get the paper background color. So from my perspective, the prop serves its purpose. However, I think that we can consider a better API/implementation for v5. Any idea? Some dimensions we could take into account:
|
|
In my opinion, |
|
Just to make it clear, which |
|
@lexskir I like the transparent proposal, I have no objection to this new value 👍. In 2018, I was doing the following: import React from 'react'
import PropTypes from 'prop-types'
import classNames from 'classnames'
import { withStyles } from '@material-ui/core/styles'
import MuiAppBar from '@material-ui/core/AppBar'
import MuiToolbar from '@material-ui/core/Toolbar'
const styles = theme => ({
root: {
boxShadow: '0 0px 20px 0px rgba(0, 0, 0, 0.05)',
paddingLeft: theme.spacing.unit * 4,
paddingRight: theme.spacing.unit * 4,
[theme.breakpoints.down('xs')]: {
paddingLeft: theme.spacing.unit * 2,
paddingRight: theme.spacing.unit * 2,
},
},
transparent: {
backgroundColor: 'transparent',
boxShadow: 'none',
'& a': {
color: theme.palette.grey[300],
'&:hover': {
color: theme.palette.common.white,
},
},
},
essential: {
boxShadow: 'none',
},
toolbar: {
padding: 0,
},
})
function AppBar(props) {
const { children, classes, essential, header, position, transparent } = props
return (
<MuiAppBar
className={classNames(classes.root, {
[classes.transparent]: transparent,
[classes.essential]: essential,
'mui-fixed': transparent,
})}
color="inherit"
elevation={0}
position={transparent ? 'absolute' : position}
>
{header}
<MuiToolbar className={classes.toolbar}>{children}</MuiToolbar>
</MuiAppBar>
)
}
AppBar.displayName = 'AppBar'
AppBar.propTypes = {
children: PropTypes.node.isRequired,
classes: PropTypes.object.isRequired,
essential: PropTypes.bool,
header: PropTypes.node,
position: PropTypes.string,
transparent: PropTypes.bool,
}
AppBar.defaultProps = {
position: 'static',
}
export default withStyles(styles)(AppBar) |
oliviertassinari
added
PR: needs revision
and removed
not planned
|
@lexskir So, do you want to introduce a new 'transparent' value for the color prop? :) |
oliviertassinari
changed the title
oliviertassinari
added
type: new feature
oliviertassinari
left a comment
oliviertassinari
left a comment
There was a problem hiding this comment.
I have tried an approach, let us know if it covers your need!
| @@ -5,7 +5,7 @@ export interface AppBarProps extends StandardProps<PaperProps, AppBarClassKey> { | |||
| /** | |||
| * The color of the component. It supports those theme colors that make sense for this component. | |||
There was a problem hiding this comment.
The description should probably be updated, since 'transparent' isn't a theme color.
There was a problem hiding this comment.
What new description do you have in mind?
There was a problem hiding this comment.
“It supports transparent and those theme colors that make sense ...”?
There was a problem hiding this comment.
inherit has the same problem, and it hasn't caused any concern so far with this or other components, so I guess we can leave it as is for now.
* Fix `AppBar color='inherit'` * Update docs * [AppBar] Add color transparent support Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
4 participants
Closes #19391