From 6f1dc4aacc270f46c82f43cdc52a25f788b41ab2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 27 Aug 2020 13:41:17 +0100 Subject: [PATCH 1/5] Fix SSR for breadcrumbs and collapsible-nav Closes #3687. Put guards around references to `window` so that in an SSR environment, EUI will still work. --- CHANGELOG.md | 4 ++- src/components/breadcrumbs/breadcrumbs.tsx | 18 +++++++---- .../collapsible_nav/collapsible_nav.tsx | 32 +++++++++++-------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc75bb745e5..1df2f81e462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -No public interface changes since `28.3.0`. +**Bug fixes** + +- Fix server-side rendering of `EuiBreadcrumbs` ([#3970](https://github.com/elastic/eui/pull/3970)) ## [`28.3.0`](https://github.com/elastic/eui/tree/v28.3.0) diff --git a/src/components/breadcrumbs/breadcrumbs.tsx b/src/components/breadcrumbs/breadcrumbs.tsx index c3140904c26..4d42ef61de6 100644 --- a/src/components/breadcrumbs/breadcrumbs.tsx +++ b/src/components/breadcrumbs/breadcrumbs.tsx @@ -187,11 +187,13 @@ export const EuiBreadcrumbs: FunctionComponent = ({ ...rest }) => { const [currentBreakpoint, setCurrentBreakpoint] = useState( - getBreakpoint(window.innerWidth) + getBreakpoint(typeof window === 'undefined' ? 1024 : window.innerWidth) ); const functionToCallOnWindowResize = throttle(() => { - const newBreakpoint = getBreakpoint(window.innerWidth); + const newBreakpoint = getBreakpoint( + typeof window === 'undefined' ? 1024 : window.innerWidth + ); if (newBreakpoint !== currentBreakpoint) { setCurrentBreakpoint(newBreakpoint); } @@ -200,11 +202,15 @@ export const EuiBreadcrumbs: FunctionComponent = ({ // Add window resize handlers useEffect(() => { - window.addEventListener('resize', functionToCallOnWindowResize); + if (typeof window !== 'undefined') { + window.addEventListener('resize', functionToCallOnWindowResize); - return () => { - window.removeEventListener('resize', functionToCallOnWindowResize); - }; + return () => { + window.removeEventListener('resize', functionToCallOnWindowResize); + }; + } else { + return () => {}; + } }, [responsive, functionToCallOnWindowResize]); const breadcrumbElements = breadcrumbs.map((breadcrumb, index) => { diff --git a/src/components/collapsible_nav/collapsible_nav.tsx b/src/components/collapsible_nav/collapsible_nav.tsx index 65e5e2d1941..b50aab2947b 100644 --- a/src/components/collapsible_nav/collapsible_nav.tsx +++ b/src/components/collapsible_nav/collapsible_nav.tsx @@ -93,14 +93,16 @@ export const EuiCollapsibleNav: FunctionComponent = ({ maskProps, ...rest }) => { + const innerWidth = typeof window === 'undefined' ? 1024 : window.innerWidth; + const [flyoutID] = useState(id || htmlIdGenerator()('euiCollapsibleNav')); const [windowIsLargeEnoughToDock, setWindowIsLargeEnoughToDock] = useState( - window.innerWidth >= dockedBreakpoint + innerWidth >= dockedBreakpoint ); const navIsDocked = isDocked && windowIsLargeEnoughToDock; const functionToCallOnWindowResize = throttle(() => { - if (window.innerWidth < dockedBreakpoint) { + if (innerWidth < dockedBreakpoint) { setWindowIsLargeEnoughToDock(false); } else { setWindowIsLargeEnoughToDock(true); @@ -110,19 +112,23 @@ export const EuiCollapsibleNav: FunctionComponent = ({ // Watch for docked status and appropriately add/remove body classes and resize handlers useEffect(() => { - window.addEventListener('resize', functionToCallOnWindowResize); + if (typeof window === 'undefined') { + return () => {}; + } else { + window.addEventListener('resize', functionToCallOnWindowResize); - if (navIsDocked) { - document.body.classList.add('euiBody--collapsibleNavIsDocked'); - } else if (isOpen) { - document.body.classList.add('euiBody--collapsibleNavIsOpen'); - } + if (navIsDocked) { + document.body.classList.add('euiBody--collapsibleNavIsDocked'); + } else if (isOpen) { + document.body.classList.add('euiBody--collapsibleNavIsOpen'); + } - return () => { - document.body.classList.remove('euiBody--collapsibleNavIsDocked'); - document.body.classList.remove('euiBody--collapsibleNavIsOpen'); - window.removeEventListener('resize', functionToCallOnWindowResize); - }; + return () => { + document.body.classList.remove('euiBody--collapsibleNavIsDocked'); + document.body.classList.remove('euiBody--collapsibleNavIsOpen'); + window.removeEventListener('resize', functionToCallOnWindowResize); + }; + } }, [navIsDocked, functionToCallOnWindowResize, isOpen]); const onKeyDown = (event: KeyboardEvent) => { From ed937b53dfe1e6c6dbf5fcddb2873c72c3907d86 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 27 Aug 2020 16:12:10 +0100 Subject: [PATCH 2/5] Use Infinity instead of arbitrary value --- src/components/breadcrumbs/breadcrumbs.tsx | 8 ++++---- src/components/collapsible_nav/collapsible_nav.tsx | 3 ++- src/services/breakpoint.ts | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/breadcrumbs/breadcrumbs.tsx b/src/components/breadcrumbs/breadcrumbs.tsx index 4d42ef61de6..bedf665db23 100644 --- a/src/components/breadcrumbs/breadcrumbs.tsx +++ b/src/components/breadcrumbs/breadcrumbs.tsx @@ -186,14 +186,14 @@ export const EuiBreadcrumbs: FunctionComponent = ({ max = 5, ...rest }) => { + const innerWidth = + typeof window === 'undefined' ? Infinity : window.innerWidth; const [currentBreakpoint, setCurrentBreakpoint] = useState( - getBreakpoint(typeof window === 'undefined' ? 1024 : window.innerWidth) + getBreakpoint(innerWidth) ); const functionToCallOnWindowResize = throttle(() => { - const newBreakpoint = getBreakpoint( - typeof window === 'undefined' ? 1024 : window.innerWidth - ); + const newBreakpoint = getBreakpoint(innerWidth); if (newBreakpoint !== currentBreakpoint) { setCurrentBreakpoint(newBreakpoint); } diff --git a/src/components/collapsible_nav/collapsible_nav.tsx b/src/components/collapsible_nav/collapsible_nav.tsx index b50aab2947b..1d4f9e34637 100644 --- a/src/components/collapsible_nav/collapsible_nav.tsx +++ b/src/components/collapsible_nav/collapsible_nav.tsx @@ -93,7 +93,8 @@ export const EuiCollapsibleNav: FunctionComponent = ({ maskProps, ...rest }) => { - const innerWidth = typeof window === 'undefined' ? 1024 : window.innerWidth; + const innerWidth = + typeof window === 'undefined' ? Infinity : window.innerWidth; const [flyoutID] = useState(id || htmlIdGenerator()('euiCollapsibleNav')); const [windowIsLargeEnoughToDock, setWindowIsLargeEnoughToDock] = useState( diff --git a/src/services/breakpoint.ts b/src/services/breakpoint.ts index 5ad1535d549..ceb251a6214 100644 --- a/src/services/breakpoint.ts +++ b/src/services/breakpoint.ts @@ -44,7 +44,7 @@ export const BREAKPOINT_KEYS = keysOf(BREAKPOINTS); * that is less than or equal to the width * * @param {number} width Can either be the full window width or any width - * @param {EuiBreakpoints} breakpoints An object with keys for sizing and values for minimu width + * @param {EuiBreakpoints} breakpoints An object with keys for sizing and values for minimum width * @returns {string | undefined} Name of the breakpoint key or `undefined` if a key doesn't exist */ export function getBreakpoint( From b2d934a4fb2b47e6c85ae73f08cac83353bf2ed2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 27 Aug 2020 16:40:34 +0100 Subject: [PATCH 3/5] Swap Infinity with -Infinity --- src/components/breadcrumbs/breadcrumbs.tsx | 2 +- src/components/collapsible_nav/collapsible_nav.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/breadcrumbs/breadcrumbs.tsx b/src/components/breadcrumbs/breadcrumbs.tsx index bedf665db23..6f74825c8bb 100644 --- a/src/components/breadcrumbs/breadcrumbs.tsx +++ b/src/components/breadcrumbs/breadcrumbs.tsx @@ -187,7 +187,7 @@ export const EuiBreadcrumbs: FunctionComponent = ({ ...rest }) => { const innerWidth = - typeof window === 'undefined' ? Infinity : window.innerWidth; + typeof window === 'undefined' ? -Infinity : window.innerWidth; const [currentBreakpoint, setCurrentBreakpoint] = useState( getBreakpoint(innerWidth) ); diff --git a/src/components/collapsible_nav/collapsible_nav.tsx b/src/components/collapsible_nav/collapsible_nav.tsx index 1d4f9e34637..767a6339ca9 100644 --- a/src/components/collapsible_nav/collapsible_nav.tsx +++ b/src/components/collapsible_nav/collapsible_nav.tsx @@ -94,7 +94,7 @@ export const EuiCollapsibleNav: FunctionComponent = ({ ...rest }) => { const innerWidth = - typeof window === 'undefined' ? Infinity : window.innerWidth; + typeof window === 'undefined' ? -Infinity : window.innerWidth; const [flyoutID] = useState(id || htmlIdGenerator()('euiCollapsibleNav')); const [windowIsLargeEnoughToDock, setWindowIsLargeEnoughToDock] = useState( From 5ec527cd45bce31ebb52208fb87ac3ef7f289ff2 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 27 Aug 2020 18:54:47 +0100 Subject: [PATCH 4/5] Back out unnecessary changes --- src/components/breadcrumbs/breadcrumbs.tsx | 22 ++++------ .../collapsible_nav/collapsible_nav.tsx | 42 ++++++++----------- 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/components/breadcrumbs/breadcrumbs.tsx b/src/components/breadcrumbs/breadcrumbs.tsx index 6f74825c8bb..29ace291a2a 100644 --- a/src/components/breadcrumbs/breadcrumbs.tsx +++ b/src/components/breadcrumbs/breadcrumbs.tsx @@ -22,8 +22,8 @@ import React, { FunctionComponent, MouseEventHandler, ReactNode, - useState, useEffect, + useState, } from 'react'; import classNames from 'classnames'; import { throttle } from '../color_picker/utils'; @@ -34,7 +34,7 @@ import { EuiInnerText } from '../inner_text'; import { EuiLink } from '../link'; import { EuiPopover } from '../popover'; import { EuiIcon } from '../icon'; -import { getBreakpoint, EuiBreakpointSize } from '../../services/breakpoint'; +import { EuiBreakpointSize, getBreakpoint } from '../../services/breakpoint'; export type EuiBreadcrumbResponsiveMaxCount = { /** @@ -186,14 +186,12 @@ export const EuiBreadcrumbs: FunctionComponent = ({ max = 5, ...rest }) => { - const innerWidth = - typeof window === 'undefined' ? -Infinity : window.innerWidth; const [currentBreakpoint, setCurrentBreakpoint] = useState( - getBreakpoint(innerWidth) + getBreakpoint(typeof window === 'undefined' ? -Infinity : window.innerWidth) ); const functionToCallOnWindowResize = throttle(() => { - const newBreakpoint = getBreakpoint(innerWidth); + const newBreakpoint = getBreakpoint(window.innerWidth); if (newBreakpoint !== currentBreakpoint) { setCurrentBreakpoint(newBreakpoint); } @@ -202,15 +200,11 @@ export const EuiBreadcrumbs: FunctionComponent = ({ // Add window resize handlers useEffect(() => { - if (typeof window !== 'undefined') { - window.addEventListener('resize', functionToCallOnWindowResize); + window.addEventListener('resize', functionToCallOnWindowResize); - return () => { - window.removeEventListener('resize', functionToCallOnWindowResize); - }; - } else { - return () => {}; - } + return () => { + window.removeEventListener('resize', functionToCallOnWindowResize); + }; }, [responsive, functionToCallOnWindowResize]); const breadcrumbElements = breadcrumbs.map((breadcrumb, index) => { diff --git a/src/components/collapsible_nav/collapsible_nav.tsx b/src/components/collapsible_nav/collapsible_nav.tsx index 767a6339ca9..c76a708cba4 100644 --- a/src/components/collapsible_nav/collapsible_nav.tsx +++ b/src/components/collapsible_nav/collapsible_nav.tsx @@ -18,17 +18,17 @@ */ import React, { + cloneElement, FunctionComponent, + HTMLAttributes, + ReactElement, ReactNode, useEffect, useState, - HTMLAttributes, - ReactElement, - cloneElement, } from 'react'; import classNames from 'classnames'; import { throttle } from '../color_picker/utils'; -import { EuiWindowEvent, keys, htmlIdGenerator } from '../../services'; +import { EuiWindowEvent, htmlIdGenerator, keys } from '../../services'; import { EuiFocusTrap } from '../focus_trap'; import { EuiOverlayMask, EuiOverlayMaskProps } from '../overlay_mask'; import { CommonProps } from '../common'; @@ -93,17 +93,15 @@ export const EuiCollapsibleNav: FunctionComponent = ({ maskProps, ...rest }) => { - const innerWidth = - typeof window === 'undefined' ? -Infinity : window.innerWidth; - const [flyoutID] = useState(id || htmlIdGenerator()('euiCollapsibleNav')); const [windowIsLargeEnoughToDock, setWindowIsLargeEnoughToDock] = useState( - innerWidth >= dockedBreakpoint + (typeof window === 'undefined' ? -Infinity : window.innerWidth) >= + dockedBreakpoint ); const navIsDocked = isDocked && windowIsLargeEnoughToDock; const functionToCallOnWindowResize = throttle(() => { - if (innerWidth < dockedBreakpoint) { + if (window.innerWidth < dockedBreakpoint) { setWindowIsLargeEnoughToDock(false); } else { setWindowIsLargeEnoughToDock(true); @@ -113,23 +111,19 @@ export const EuiCollapsibleNav: FunctionComponent = ({ // Watch for docked status and appropriately add/remove body classes and resize handlers useEffect(() => { - if (typeof window === 'undefined') { - return () => {}; - } else { - window.addEventListener('resize', functionToCallOnWindowResize); - - if (navIsDocked) { - document.body.classList.add('euiBody--collapsibleNavIsDocked'); - } else if (isOpen) { - document.body.classList.add('euiBody--collapsibleNavIsOpen'); - } + window.addEventListener('resize', functionToCallOnWindowResize); - return () => { - document.body.classList.remove('euiBody--collapsibleNavIsDocked'); - document.body.classList.remove('euiBody--collapsibleNavIsOpen'); - window.removeEventListener('resize', functionToCallOnWindowResize); - }; + if (navIsDocked) { + document.body.classList.add('euiBody--collapsibleNavIsDocked'); + } else if (isOpen) { + document.body.classList.add('euiBody--collapsibleNavIsOpen'); } + + return () => { + document.body.classList.remove('euiBody--collapsibleNavIsDocked'); + document.body.classList.remove('euiBody--collapsibleNavIsOpen'); + window.removeEventListener('resize', functionToCallOnWindowResize); + }; }, [navIsDocked, functionToCallOnWindowResize, isOpen]); const onKeyDown = (event: KeyboardEvent) => { From b63e4d2bffb6ab6fd117fe90bb85deff8981cc0b Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 1 Sep 2020 08:42:45 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> --- CHANGELOG.md | 2 +- src/components/collapsible_nav/collapsible_nav.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88822e54f77..64b43de1f5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ **Bug fixes** - Fixed bug in `EuiComboBox` where the input was dropping to the next line when a `EuiBadge` had a very long text ([#3968](https://github.com/elastic/eui/pull/3968)) -- Fix server-side rendering of `EuiBreadcrumbs` ([#3970](https://github.com/elastic/eui/pull/3970)) +- Fix server-side rendering of `EuiBreadcrumbs` and `EuiCollapsibleNav` ([#3970](https://github.com/elastic/eui/pull/3970)) ## [`28.3.0`](https://github.com/elastic/eui/tree/v28.3.0) diff --git a/src/components/collapsible_nav/collapsible_nav.tsx b/src/components/collapsible_nav/collapsible_nav.tsx index c76a708cba4..4d5ebf23931 100644 --- a/src/components/collapsible_nav/collapsible_nav.tsx +++ b/src/components/collapsible_nav/collapsible_nav.tsx @@ -95,7 +95,7 @@ export const EuiCollapsibleNav: FunctionComponent = ({ }) => { const [flyoutID] = useState(id || htmlIdGenerator()('euiCollapsibleNav')); const [windowIsLargeEnoughToDock, setWindowIsLargeEnoughToDock] = useState( - (typeof window === 'undefined' ? -Infinity : window.innerWidth) >= + (typeof window === 'undefined' ? Infinity : window.innerWidth) >= dockedBreakpoint ); const navIsDocked = isDocked && windowIsLargeEnoughToDock;