-
Notifications
You must be signed in to change notification settings - Fork 23
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
SWC-7166 - Fix OrientationBanner display & add spreadSx utility #1463
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,13 @@ | ||
import { useTheme } from '@mui/material' | ||
import { Theme } from '@mui/material' | ||
import { SxProps } from '@mui/system' | ||
import { ReactNode, useState } from 'react' | ||
import Illustrations from '../../assets/illustrations' | ||
import { spreadSx } from '../../theme/spreadSx' | ||
import { useCookiePreferences } from '../../utils/hooks/useCookiePreferences' | ||
import FullWidthAlert, { | ||
AlertButtonConfig, | ||
FullWidthAlertProps, | ||
} from '../FullWidthAlert/FullWidthAlert' | ||
import Illustrations from '../../assets/illustrations' | ||
import { useCookiePreferences } from '../../utils/hooks/useCookiePreferences' | ||
import { ReactNode, useState } from 'react' | ||
|
||
const OrientationBannerNameStrings = [ | ||
'Challenges', | ||
|
@@ -54,9 +56,10 @@ function OrientationBanner(props: OrientationBannerProps) { | |
localStorage.getItem(storageBannerId) === null, | ||
) | ||
|
||
const theme = useTheme() | ||
const defaultSx = { | ||
display: { xs: 'none', md: 'unset' }, | ||
const defaultSx: SxProps<Theme> = theme => ({ | ||
[theme.breakpoints.down('md')]: { | ||
display: 'none', | ||
}, | ||
backgroundColor: '#F9FAFB', | ||
Comment on lines
+60
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More concise/conventional way to just change the breakpoint styles is to do this. Now we aren't touching md and up. |
||
border: 'none', | ||
boxShadow: '0px 4px 4px rgba(0, 0, 0, 0.05)', | ||
|
@@ -65,7 +68,7 @@ function OrientationBanner(props: OrientationBannerProps) { | |
paddingLeft: 4, | ||
'.MuiAlert-icon': { mr: 5 }, | ||
'.MuiAlertTitle-root': { color: theme.palette.grey[1000] }, | ||
} | ||
}) | ||
const BannerIllustration = Illustrations[name] | ||
|
||
return ( | ||
|
@@ -83,10 +86,7 @@ function OrientationBanner(props: OrientationBannerProps) { | |
} | ||
setShowBanner(false) | ||
}} | ||
sx={{ | ||
...defaultSx, | ||
...sx, | ||
}} | ||
sx={spreadSx(defaultSx, sx)} | ||
/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I learned that you can pass an array of SxProps to |
||
) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { SxProps, Theme } from '@mui/material' | ||
import { spreadSx } from './spreadSx' | ||
|
||
describe('spreadSx', () => { | ||
const sxObject: SxProps<Theme> = { color: 'red' } | ||
const nestedSxObject: SxProps<Theme> = [{ color: 'blue' }] | ||
const sxFunction: SxProps<Theme> = theme => ({ | ||
color: theme.palette.primary.main, | ||
}) | ||
const nestedSxFunction: SxProps<Theme> = [ | ||
theme => ({ color: theme.palette.secondary.main }), | ||
] | ||
|
||
it('Removes undefined sx', () => { | ||
expect(spreadSx(sxObject, undefined)).toEqual([sxObject]) | ||
}) | ||
it('Flattens a nested array', () => { | ||
expect(spreadSx(sxObject, nestedSxObject)).toEqual([ | ||
sxObject, | ||
nestedSxObject[0], | ||
]) | ||
}) | ||
it('Handles a mix of functions and objects', () => { | ||
expect( | ||
spreadSx( | ||
sxObject, | ||
nestedSxObject, | ||
sxFunction, | ||
nestedSxFunction, | ||
undefined, | ||
), | ||
).toEqual([sxObject, nestedSxObject[0], sxFunction, nestedSxFunction[0]]) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { Theme } from '@mui/material' | ||
import { SxProps } from '@mui/system' | ||
|
||
/** | ||
* Utility to combine multiple SxProps into a single SxProps object that can be passed to a component. | ||
* | ||
* See https://github.com/mui/material-ui/issues/29274#issuecomment-953980228 | ||
* @param sxProps | ||
*/ | ||
export function spreadSx( | ||
...sxProps: (SxProps<Theme> | undefined)[] | ||
): SxProps<Theme> { | ||
return sxProps.filter(sx => sx !== undefined).flat() | ||
} |
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.
Setting this to
unset
ended up replacing theflex
layout, so we had to removeunset
somehow