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

[AppBar] Fix background color on dark mode #26545

Merged
merged 13 commits into from
Jun 3, 2021
1 change: 1 addition & 0 deletions docs/pages/api-docs/app-bar.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
},
"default": "'primary'"
},
"enableColorOnDark": { "type": { "name": "bool" } },
"position": {
"type": {
"name": "enum",
Expand Down
2 changes: 0 additions & 2 deletions docs/src/modules/components/AppFrame.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ const styles = (theme) => ({
},
},
appBar: {
color: theme.palette.mode === 'light' ? null : '#fff',
backgroundColor: theme.palette.mode === 'light' ? null : theme.palette.background.level2,
transition: theme.transitions.create('width'),
},
language: {
Expand Down
24 changes: 24 additions & 0 deletions docs/src/pages/components/app-bar/app-bar.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,27 @@ function HideOnScroll(props) {
);
}
```

## Enable Color on Dark
Copy link
Member

@oliviertassinari oliviertassinari Jun 3, 2021

Choose a reason for hiding this comment

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

Our heading capitalization convention:

Suggested change
## Enable Color on Dark
## Enable color on dark

Copy link
Member

Choose a reason for hiding this comment

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

We might want to explain what "on dark" mean

Suggested change
## Enable Color on Dark
## Enable color on dark mode


From v5 onward, `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html). You can opt out by passing `enableColorOnDark` prop.
Copy link
Member

Choose a reason for hiding this comment

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

Prefer breaking lines between sentences, makes reviewing easier:

Suggested change
From v5 onward, `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html). You can opt out by passing `enableColorOnDark` prop.
From v5 onward, `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html).
You can opt out by passing `enableColorOnDark` prop.

Copy link
Member

Choose a reason for hiding this comment

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

We usually don't mention previous versions:

Suggested change
From v5 onward, `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html). You can opt out by passing `enableColorOnDark` prop.
The `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html).
You can opt out by passing `enableColorOnDark` prop.

Copy link
Member

Choose a reason for hiding this comment

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

@mbrookes Are you OK to settle on this terminology?

Suggested change
From v5 onward, `color` prop has no effect on dark mode according to [material design spec](https://material.io/design/color/dark-theme.html). You can opt out by passing `enableColorOnDark` prop.
The `color` prop has no effect on dark mode according to [Material Design guidelines](https://material.io/design/color/dark-theme.html).
You can opt out by passing `enableColorOnDark` prop.

Copy link
Member

Choose a reason for hiding this comment

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

@mbrookes Are you OK to settle on this terminology?

No, but it seems that it was merged before you asked 🙂.

A gentle reminder to @mui-org/maintainers to tag me on any PRs with new or significantly changed language or terminology.

Copy link
Member

Choose a reason for hiding this comment

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

@mbrookes You have another shoot at it in #26628.


```jsx
// Specific element via prop
<AppBar enableColorOnDark />

// Affect all AppBars via theme
<ThemeProvider
theme={createTheme({
components: {
MuiAppBar: {
defaultProps: {
enableColorOnDark: true,
},
},
},
})}
>
<AppBar />
</ThemeProvider>
```
7 changes: 6 additions & 1 deletion docs/src/pages/guides/migration-v4/migration-v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,12 @@ As the core components use emotion as their style engine, the props used by emot

### AppBar

- [AppBar] Remove z-index when position static and relative. This avoids the creation of a stacking context and rendering issues.
- Remove z-index when position static and relative. This avoids the creation of a stacking context and rendering issues.
- The `color` prop has no longer any effect in dark mode. The app bar uses the background color required by the elevation to follow the [Material Design guidelines](https://material.io/design/color/dark-theme.html). Use `enableColorOnDark` to restore the behavior of v4.

```jsx
<AppBar enableColorOnDark />
```

### Alert

Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs/app-bar/app-bar.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"children": "The content of the component.",
"classes": "Override or extend the styles applied to the component. See <a href=\"#css\">CSS API</a> below for more details.",
"color": "The color of the component. It supports those theme colors that make sense for this component.",
"enableColorOnDark": "If true, the <code>color</code> prop is applied in dark mode.",
"position": "The positioning type. The behavior of the different options is described <a href=\"https://developer.mozilla.org/en-US/docs/Learn/CSS/CSS_layout/Positioning\">in the MDN web docs</a>. Note: <code>sticky</code> is not universally supported and will fall back to <code>static</code> when unavailable.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/basics/#the-sx-prop\">`sx` page</a> for more details."
},
Expand Down
5 changes: 5 additions & 0 deletions packages/material-ui/src/AppBar/AppBar.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export interface AppBarTypeMap<P = {}, D extends React.ElementType = 'header'> {
* @default 'primary'
*/
color?: OverridableStringUnion<PropTypes.Color | 'transparent', AppBarPropsColorOverrides>;
/**
* If true, the `color` prop is applied in dark mode.
* @default false
*/
enableColorOnDark?: boolean;
/**
* The positioning type. The behavior of the different options is described
* [in the MDN web docs](https://developer.mozilla.org/en-US/docs/Learn/CSS/CSS_layout/Positioning).
Expand Down
19 changes: 18 additions & 1 deletion packages/material-ui/src/AppBar/AppBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,29 @@ const AppBarRoot = experimentalStyled(Paper, {
backgroundColor: 'transparent',
color: 'inherit',
}),
...(theme.palette.mode === 'dark' &&
!styleProps.enableColorOnDark && {
backgroundColor: null,
color: null,
}),
};
});

const AppBar = React.forwardRef(function AppBar(inProps, ref) {
const props = useThemeProps({ props: inProps, name: 'MuiAppBar' });
const { className, color = 'primary', position = 'fixed', ...other } = props;
const {
className,
color = 'primary',
enableColorOnDark = false,
position = 'fixed',
...other
} = props;

const styleProps = {
...props,
color,
position,
enableColorOnDark,
};

const classes = useUtilityClasses(styleProps);
Expand Down Expand Up @@ -159,6 +171,11 @@ AppBar.propTypes /* remove-proptypes */ = {
PropTypes.oneOf(['default', 'inherit', 'primary', 'secondary', 'transparent']),
PropTypes.string,
]),
/**
* If true, the `color` prop is applied in dark mode.
* @default false
*/
enableColorOnDark: PropTypes.bool,
/**
* The positioning type. The behavior of the different options is described
* [in the MDN web docs](https://developer.mozilla.org/en-US/docs/Learn/CSS/CSS_layout/Positioning).
Expand Down