Skip to content

Add variant prop to AppBar with relative, fixed, and hide values. The… #94

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

Merged
merged 2 commits into from
Jun 3, 2020
Merged

Add variant prop to AppBar with relative, fixed, and hide values. The… #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd0f018
Add variant prop to AppBar with relative, fixed, and hide values. The…
markbrocato Jun 2, 2020
c1f5d49
some cleanup
markbrocato Jun 2, 2020
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
31 changes: 26 additions & 5 deletions src/AppBar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { makeStyles } from '@material-ui/core/styles'
import { AppBar as MUIAppBar, Toolbar, useScrollTrigger, Slide } from '@material-ui/core'
import PropTypes from 'prop-types'
import PWAContext from './PWAContext'
import clsx from 'clsx'

const useStyles = makeStyles(theme => ({
/**
Expand All @@ -19,6 +20,9 @@ const useStyles = makeStyles(theme => ({
flexDirection: 'column',
alignItems: 'stretch',
},
relative: {
position: 'relative',
},
/**
* Styles applied to the spacer that fills the height behind the floating toolbar.
*/
Expand Down Expand Up @@ -46,15 +50,22 @@ const useStyles = makeStyles(theme => ({
},
}))

export default function AppBar({ children, style, fixed, offlineWarning, classes }) {
export default function AppBar({ children, style, variant, fixed, offlineWarning, classes }) {
if (fixed) {
variant = 'fixed'
}

const trigger = useScrollTrigger()
classes = useStyles({ classes })

const { offline } = useContext(PWAContext)

let appBar = (
<MUIAppBar
className={classes.root}
className={clsx({
[classes.root]: true,
[classes.relative]: variant === 'relative',
})}
style={{
...style,
}}
Expand All @@ -65,7 +76,7 @@ export default function AppBar({ children, style, fixed, offlineWarning, classes
</MUIAppBar>
)

if (!fixed) {
if (variant === 'hide') {
appBar = (
<Slide appear={false} in={!trigger}>
{appBar}
Expand All @@ -75,7 +86,7 @@ export default function AppBar({ children, style, fixed, offlineWarning, classes

return (
<>
<div className={classes.spacer} />
{(variant === 'hide' || variant === 'fixed') && <div className={classes.spacer} />}
{offline && <div className={classes.offline}>{offlineWarning}</div>}
{appBar}
</>
Expand All @@ -89,17 +100,27 @@ AppBar.propTypes = {
classes: PropTypes.object,

/**
* Set as `true` if the AppBar should be fixed position.
* Affixes the AppBar to the top of the viewport. This prop is deprecated.
* Use `variant="fixed"` instead.
* @deprecated
*/
fixed: PropTypes.bool,

/**
* String or Element to render within the offline warning container at the top of the app.
*/
offlineWarning: PropTypes.oneOfType([PropTypes.string, PropTypes.element]),

/**
* * relative - The AppBar stays at the top of the page.
Copy link
Contributor

@dijs dijs Jun 3, 2020

Choose a reason for hiding this comment

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

Did you mean to leave these extra asterisks in? I think they might break the documentation parsing.

Copy link
Contributor Author

@markbrocato markbrocato Jun 3, 2020

Choose a reason for hiding this comment

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

Yes, I did - I think that will create a markdown list, but I'm not sure. Will check after release.

Copy link
Contributor

@dijs dijs Jun 3, 2020

Choose a reason for hiding this comment

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

Sweet

* * fixed - The AppBar stays at the top of the viewport.
* * hide - The same as fixed, but the app bar automatically hides when the user scrolls down.
*/
variant: PropTypes.oneOf(['relative', 'fixed', 'hide']),
}

AppBar.defaultProps = {
offlineWarning: 'Your device lost its internet connection.',
variant: 'hide',
fixed: false,
}
9 changes: 7 additions & 2 deletions test/AppBar.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ jest.useFakeTimers()
describe('AppBar', () => {
let wrapper

const Test = ({ offline = false, offlineMessage, fixed = false }) => {
const Test = ({ offline = false, offlineMessage, ...others }) => {
return (
<PWAContext.Provider value={{ offline }}>
<MuiThemeProvider theme={theme}>
<AppBar offlineWarning={offlineMessage} fixed={fixed} />
<AppBar offlineWarning={offlineMessage} {...others} />
</MuiThemeProvider>
</PWAContext.Provider>
)
Expand Down Expand Up @@ -46,6 +46,11 @@ describe('AppBar', () => {
expect(wrapper.find(Slide)).toHaveLength(0)
})

it('should accept variant="relative"', () => {
wrapper = mount(<Test variant="relative" />)
expect(wrapper.find(Slide)).toHaveLength(0)
})

// it('should not have any classes except wrap when not scrolled', async () => {
// window.scrollY = 0

Expand Down