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.

Sign up for GitHub

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

Jump to bottom

[docs] document use of theme.mixins.toolbar & <Toolbar /> when using Appbar variant fixed #17878

Merged
merged 3 commits into from
Oct 15, 2019
Merged

[docs] document use of theme.mixins.toolbar & <Toolbar /> when using Appbar variant fixed #17878

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44692a4
document use of theme.mixins.toolbar & <Toolbar /> when using Appbar …
adeelibr Oct 14, 2019
ab39334
document improve
adeelibr Oct 14, 2019
832493a
remove extra line
adeelibr Oct 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/src/pages/components/app-bar/app-bar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,33 @@ A side searchbar.

You can use the `useScrollTrigger()` hook to respond to user scroll actions.

## Placement

When using Appbar `variant="fixed"` you need to have extra space for the content to show below
& not under. There are 2 ways to do it. Either use `theme.mixins.toolbar` like;
Copy link
Member

Choose a reason for hiding this comment

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

Do we tend to use & in the docs @mbrookes ?

Copy link
Member

Choose a reason for hiding this comment

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

That semicolon should be a colon right?


```jsx
const useStyles = makeStyles(theme => ({
offset: {
...theme.mixins.toolbar,
flexGrow: 1
Copy link
Member

Choose a reason for hiding this comment

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

Do we need flexGrow in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no flexGrow is not needed, I'll update the PR

}
}))

const App = () => {
Copy link
Member

Choose a reason for hiding this comment

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

function App() {?

const classes = useStyles();
return (
<div>
<Appbar position="fixed"></Appbar>
<div className={classes.offset}> {/* to accomdate for top white space */}
</div>
)
};
```

Or you can append `<Toolbar />` component after `<Appbar />` like shown in the example
below. To prevent content from hiding under Appbar.
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of adding an plain example (to remove and source of doubt and to avoid having to dig in the following demo)?


### Hide App Bar

The app bar hides on scroll down to leave more space for reading.
Expand Down