-
Notifications
You must be signed in to change notification settings - Fork 839
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
[EuiCollapsibleNavBeta] Final collapsed/docked icon & popover behavior #7034
Changes from 13 commits
5dff814
b540a06
5b7869b
b19eddc
5e22be8
1cff317
0c3cb8d
965b476
0fce843
17b8c62
f9838a3
d6c7f7e
a582ddc
2a1ec67
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 |
---|---|---|
|
@@ -8,13 +8,26 @@ | |
|
||
import { css } from '@emotion/react'; | ||
import { UseEuiTheme } from '../../services'; | ||
import { logicalCSS } from '../../global_styling'; | ||
import { logicalCSS, euiYScroll } from '../../global_styling'; | ||
import { euiShadowFlat } from '../../themes'; | ||
|
||
export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { | ||
const { euiTheme } = euiThemeContext; | ||
|
||
return { | ||
euiCollapsibleNavBeta: css` | ||
/* This extra padding is needed for EuiPopovers to have enough | ||
space to render with the right anchorPosition */ | ||
${logicalCSS('padding-bottom', euiTheme.size.xs)} | ||
|
||
/* Allow the nav to scroll, in case consumers don't use EuiFlyoutBody/EuiFyoutFooter */ | ||
${euiYScroll(euiThemeContext)} | ||
|
||
/* In case things get really dire responsively, ensure the footer doesn't overtake the body */ | ||
.euiFlyoutBody { | ||
${logicalCSS('min-height', '50%')} | ||
} | ||
|
||
Comment on lines
+27
to
+30
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. Good call on this. I toyed around by opening all of the toggles between the flyout body and footer. Hopefully no one would do this, but this is nice just in case. 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. Haha, I remembered that one issue/bug report we got previously and tried to get ahead of it! :) |
||
.euiFlyoutFooter { | ||
background-color: ${euiTheme.colors.emptyShade}; | ||
${logicalCSS('border-top', euiTheme.border.thin)} | ||
|
@@ -26,7 +39,22 @@ export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { | |
right: css` | ||
${logicalCSS('border-left', euiTheme.border.thin)} | ||
`, | ||
isSmallestScreen: css` | ||
isPush: css` | ||
${euiShadowFlat(euiThemeContext)} | ||
`, | ||
isPushCollapsed: css` | ||
/* Hide the scrollbar for docked mode (while still keeping the nav scrollable) | ||
Otherwise if scrollbars are visible, button icon visibility suffers */ | ||
&, | ||
.euiFlyoutBody__overflow { | ||
scrollbar-width: none; /* Firefox */ | ||
|
||
&::-webkit-scrollbar { | ||
display: none; /* Chrome, Edge, & Safari */ | ||
} | ||
} | ||
`, | ||
isOverlayFullWidth: css` | ||
/* Override EuiFlyout's max-width */ | ||
&.euiFlyout { | ||
${logicalCSS('max-width', '100% !important')} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ import { CommonProps } from '../common'; | |
import { EuiFlyout, EuiFlyoutProps } from '../flyout'; | ||
import { euiHeaderVariables } from '../header/header.styles'; | ||
|
||
import { EuiCollapsibleNavContext } from './context'; | ||
import { EuiCollapsibleNavButton } from './collapsible_nav_button'; | ||
import { euiCollapsibleNavBetaStyles } from './collapsible_nav_beta.styles'; | ||
|
||
|
@@ -87,16 +88,21 @@ export const EuiCollapsibleNavBeta: FunctionComponent< | |
const onClose = useCallback(() => setIsCollapsed(true), []); | ||
|
||
/** | ||
* Mobile behavior | ||
* Responsive behavior | ||
* By default on large enough screens, the nav is always a push flyout, | ||
* but on smaller/mobile screens, the nav overlays the page instead | ||
*/ | ||
const [isSmallScreen, setIsSmallScreen] = useState(false); | ||
const [isSmallestScreen, setIsSmallestScreen] = useState(false); | ||
const [isOverlay, setIsOverlay] = useState(false); | ||
const [isOverlayFullWidth, setIsOverlayFullWidth] = useState(false); | ||
Comment on lines
+95
to
+96
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 like this change! It's very intuitive. 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. Music to my ears!! 😁 |
||
|
||
// Add a window resize listener that determines breakpoint behavior | ||
const flyoutType = isOverlay ? 'overlay' : 'push'; | ||
const isPush = !isOverlay; | ||
|
||
// Set up a window resize listener that determines breakpoint behavior | ||
useEffect(() => { | ||
const getBreakpoints = () => { | ||
setIsSmallScreen(window.innerWidth < _width * 3); | ||
setIsSmallestScreen(window.innerWidth < _width * 1.5); | ||
setIsOverlay(window.innerWidth < _width * 3); | ||
setIsOverlayFullWidth(window.innerWidth < _width * 1.5); | ||
}; | ||
getBreakpoints(); | ||
|
||
|
@@ -105,22 +111,17 @@ export const EuiCollapsibleNavBeta: FunctionComponent< | |
return () => window.removeEventListener('resize', onWindowResize); | ||
}, [_width]); | ||
|
||
// If the screen was previously uncollapsed and shrinks down to | ||
// a smaller mobile view, default that view to a collapsed state | ||
// If the nav was previously uncollapsed and shrinks down to the | ||
// overlay flyout, default to its hidden/collapsed state | ||
useEffect(() => { | ||
if (isSmallScreen) setIsCollapsed(true); | ||
}, [isSmallScreen]); | ||
|
||
// On small screens, the flyout becomes an overlay rather than a push | ||
const flyoutType = isSmallScreen ? 'overlay' : 'push'; | ||
const isMobileCollapsed = isSmallScreen && isCollapsed; | ||
if (isOverlay) setIsCollapsed(true); | ||
}, [isOverlay]); | ||
|
||
const width = useMemo(() => { | ||
if (isSmallestScreen) return '100%'; | ||
if (isSmallScreen) return _width; | ||
if (isCollapsed) return headerHeight; | ||
if (isOverlayFullWidth) return '100%'; | ||
if (isPush && isCollapsed) return headerHeight; | ||
return _width; | ||
}, [_width, isSmallScreen, isSmallestScreen, isCollapsed, headerHeight]); | ||
}, [_width, isOverlayFullWidth, isPush, isCollapsed, headerHeight]); | ||
|
||
/** | ||
* Header affordance | ||
|
@@ -174,7 +175,9 @@ export const EuiCollapsibleNavBeta: FunctionComponent< | |
const cssStyles = [ | ||
styles.euiCollapsibleNavBeta, | ||
styles[side], | ||
isSmallestScreen && styles.isSmallestScreen, | ||
isPush && styles.isPush, | ||
isPush && isCollapsed && styles.isPushCollapsed, | ||
isOverlayFullWidth && styles.isOverlayFullWidth, | ||
]; | ||
|
||
// Wait for any fixed headers to be queried before rendering (prevents position jumping) | ||
|
@@ -199,18 +202,16 @@ export const EuiCollapsibleNavBeta: FunctionComponent< | |
</EuiFlyout> | ||
); | ||
|
||
const hideFlyout = isOverlay && isCollapsed; | ||
|
||
return ( | ||
// TODO: Context for sharing state to all children | ||
<> | ||
<EuiCollapsibleNavContext.Provider value={{ isPush, isCollapsed, side }}> | ||
<EuiCollapsibleNavButton | ||
ref={buttonRef} | ||
onClick={toggleCollapsed} | ||
isCollapsed={isCollapsed} | ||
isSmallScreen={isSmallScreen} | ||
side={side} | ||
aria-controls={flyoutID} | ||
/> | ||
{!isMobileCollapsed && flyout} | ||
</> | ||
{!hideFlyout && flyout} | ||
</EuiCollapsibleNavContext.Provider> | ||
); | ||
}; |
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.
Otherwise we get this:
When we want this: