From 275df48c48ab7e409924f457f31774196a197f0e Mon Sep 17 00:00:00 2001 From: MarcusNotheis Date: Wed, 22 Jan 2020 14:36:32 +0100 Subject: [PATCH 1/2] fix(ObjectPage): Don't crash when conditional rendering is used for children --- .../components/ObjectPage/ObjectPage.jss.ts | 41 +-- .../ObjectPage/ObjectPageAnchorButton.tsx | 2 +- .../ObjectPage/ObjectPageHeader.tsx | 101 ++++++ .../ObjectPage/ObjectPageScrollBar.tsx | 59 ++++ .../components/ObjectPage/ObjectPageUtils.ts | 30 ++ .../main/src/components/ObjectPage/index.tsx | 296 ++++++------------ tsconfig.json | 3 +- typings/react-jss.d.ts | 13 + 8 files changed, 304 insertions(+), 241 deletions(-) create mode 100644 packages/main/src/components/ObjectPage/ObjectPageHeader.tsx create mode 100644 packages/main/src/components/ObjectPage/ObjectPageScrollBar.tsx create mode 100644 packages/main/src/components/ObjectPage/ObjectPageUtils.ts create mode 100644 typings/react-jss.d.ts diff --git a/packages/main/src/components/ObjectPage/ObjectPage.jss.ts b/packages/main/src/components/ObjectPage/ObjectPage.jss.ts index 6db47c0ca57..f98f44d27cb 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.jss.ts +++ b/packages/main/src/components/ObjectPage/ObjectPage.jss.ts @@ -33,7 +33,8 @@ const styles = ({ parameters }: JSSTheme) => ({ boxShadow: `inset 0 -0.0625rem ${parameters.sapUiObjectHeaderBorderColor}, inset 0 0.0625rem ${parameters.sapUiObjectHeaderBorderColor}`, display: 'flex', height: '2.75rem', - minHeight: '2.75rem' + minHeight: '2.75rem', + position: 'relative' }, sectionsContainer: { '&:before': { @@ -58,32 +59,6 @@ const styles = ({ parameters }: JSSTheme) => ({ fillerDiv: { backgroundColor: parameters.sapUiBaseBG }, - outerScrollbar: { - position: 'absolute', - right: 0, - overflow: 'hidden', - height: '100%', - zIndex: ZIndex.ResponsivePopover, - backgroundColor: parameters.sapUiObjectHeaderBackground, - '& ::-webkit-scrollbar': { - backgroundColor: '#ffffff' - }, - '& ::-webkit-scrollbar-thumb': { - backgroundColor: '#949494', - '&:hover': { - backgroundColor: '#8c8c8c' - } - }, - '& ::-webkit-scrollbar-corner': { - backgroundColor: '#ffffff' - } - }, - innerScrollbar: { - width: '34px', - overflowY: 'scroll', - overflowX: 'hidden', - height: '100%' - }, // header header: { flexShrink: 0, @@ -216,18 +191,6 @@ const styles = ({ parameters }: JSSTheme) => ({ display: 'flex', flexDirection: 'row' }, - flexBoxRow: { - display: 'flex', - flexDirection: 'row' - }, - flexBoxColumn: { - display: 'flex', - flexDirection: 'column' - }, - flexBoxCenter: { - display: 'flex', - alignItems: 'center' - }, avatar: { marginRight: '1rem' } diff --git a/packages/main/src/components/ObjectPage/ObjectPageAnchorButton.tsx b/packages/main/src/components/ObjectPage/ObjectPageAnchorButton.tsx index be3c8ebbf18..912719c5672 100644 --- a/packages/main/src/components/ObjectPage/ObjectPageAnchorButton.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPageAnchorButton.tsx @@ -136,7 +136,7 @@ export const ObjectPageAnchorButton: FC = (props) => onSetActive={onScrollActive} activeClass={classes.selected} alwaysToTop={index === 0} - scrollOffset={45} + scrollOffset={collapsedHeader ? 45 : -45} > {section.props.title} diff --git a/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx b/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx new file mode 100644 index 00000000000..27afa52eb19 --- /dev/null +++ b/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx @@ -0,0 +1,101 @@ +import { AvatarSize } from '@ui5/webcomponents-react/lib/AvatarSize'; +import { FlexBox } from '@ui5/webcomponents-react/lib/FlexBox'; +import { FlexBoxDirection } from '@ui5/webcomponents-react/lib/FlexBoxDirection'; +import React, { CSSProperties, FC } from 'react'; +import { safeGetChildrenArray } from './ObjectPageUtils'; + +interface Props { + image: any; + classes: any; + imageShapeCircle: any; + showTitleInHeaderContent: any; + renderHeaderContentProp: any; + renderBreadcrumbs: any; + renderKeyInfos: any; + title: any; + subTitle: any; +} + +const positionRelativeStyle: CSSProperties = { position: 'relative' }; + +export const ObjectPageHeader: FC = (props) => { + const { + image, + classes, + imageShapeCircle, + showTitleInHeaderContent, + renderHeaderContentProp, + renderBreadcrumbs, + title, + subTitle, + renderKeyInfos + } = props; + + let avatar = null; + + if (image) { + if (typeof image === 'string') { + avatar = ( + + Company Logo + + ); + } else { + avatar = React.cloneElement(image, { + size: AvatarSize.L, + className: image.props?.className ? `${classes.headerImage} ${image.props?.className}` : classes.headerImage + } as unknown); + } + } + + if (showTitleInHeaderContent) { + const headerContents = renderHeaderContentProp && renderHeaderContentProp(); + let firstElement; + let contents = []; + + if (headerContents?.type === React.Fragment) { + [firstElement, ...contents] = safeGetChildrenArray(headerContents.props.children); + } else { + firstElement = headerContents; + } + return ( +
+
+ + {avatar} + +
{renderBreadcrumbs && renderBreadcrumbs()}
+ + +

{title}

+ {subTitle} + {firstElement} + + + {contents.map((c, index) => ( +
+ {c} +
+ ))} + +
{renderKeyInfos && renderKeyInfos()}
+ + + +
+
+ ); + } + + return ( +
+
+ {avatar} + {renderHeaderContentProp && {renderHeaderContentProp()}} +
+
+ ); +}; diff --git a/packages/main/src/components/ObjectPage/ObjectPageScrollBar.tsx b/packages/main/src/components/ObjectPage/ObjectPageScrollBar.tsx new file mode 100644 index 00000000000..fe4d58194a9 --- /dev/null +++ b/packages/main/src/components/ObjectPage/ObjectPageScrollBar.tsx @@ -0,0 +1,59 @@ +import React, { RefObject, FC, useMemo } from 'react'; +import { ZIndex } from '../../enums/ZIndex'; +import { JSSTheme } from '../../interfaces/JSSTheme'; +import { createUseStyles } from 'react-jss'; + +interface Props { + scrollBarRef: RefObject; + innerScrollBarRef: RefObject; + width: number; +} + +const styles = ({ parameters }: JSSTheme) => ({ + outerScrollbar: { + position: 'absolute', + right: 0, + overflow: 'hidden', + height: '100%', + zIndex: ZIndex.ResponsivePopover, + backgroundColor: parameters.sapUiObjectHeaderBackground, + '& ::-webkit-scrollbar': { + backgroundColor: '#ffffff' + }, + '& ::-webkit-scrollbar-thumb': { + backgroundColor: '#949494', + '&:hover': { + backgroundColor: '#8c8c8c' + } + }, + '& ::-webkit-scrollbar-corner': { + backgroundColor: '#ffffff' + } + }, + innerScrollbar: { + width: '34px', + overflowY: 'scroll', + overflowX: 'hidden', + height: '100%' + } +}); + +const useScrollBarStyles = createUseStyles(styles); + +export const ObjectPageScrollBar: FC = (props) => { + const { scrollBarRef, innerScrollBarRef, width } = props; + + const [scrollBarWidthStyle, scrollBarWidthMargin] = useMemo(() => { + return [{ width: `${width}px` }, { marginLeft: `-${width}px`, width: `${2 * width}px` }]; + }, [width]); + + const classes = useScrollBarStyles(); + + return ( +
+
+
+
+
+ ); +}; diff --git a/packages/main/src/components/ObjectPage/ObjectPageUtils.ts b/packages/main/src/components/ObjectPage/ObjectPageUtils.ts new file mode 100644 index 00000000000..43e2fd70ebe --- /dev/null +++ b/packages/main/src/components/ObjectPage/ObjectPageUtils.ts @@ -0,0 +1,30 @@ +import { Children, ReactElement } from 'react'; + +export const safeGetChildrenArray = (children) => Children.toArray(children).filter(Boolean); + +export const findSectionIndexById = (sections: ReactElement | Array>, id) => { + const index = safeGetChildrenArray(sections).findIndex((objectPageSection) => objectPageSection.props?.id === id); + if (index === -1) { + return 0; + } + return index; +}; + +export const getProportionateScrollTop = (activeContainer, passiveContainer, base) => { + const activeHeight = activeContainer.current.getBoundingClientRect().height; + const passiveHeight = passiveContainer.current.getBoundingClientRect().height; + + return (base / activeHeight) * passiveHeight; +}; + +export const bindScrollEvent = (scrollContainer, handler) => { + if (scrollContainer.current && handler.current) { + scrollContainer.current.addEventListener('scroll', handler.current, { passive: true }); + } +}; + +export const removeScrollEvent = (scrollContainer, handler) => { + if (scrollContainer.current && handler.current) { + scrollContainer.current.removeEventListener('scroll', handler.current); + } +}; diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index 4ba72669355..2ad1933f249 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/packages/main/src/components/ObjectPage/index.tsx @@ -7,23 +7,20 @@ import { StyleClassHelper } from '@ui5/webcomponents-react-base/lib/StyleClassHe import { useConsolidatedRef } from '@ui5/webcomponents-react-base/lib/useConsolidatedRef'; import { usePassThroughHtmlProps } from '@ui5/webcomponents-react-base/lib/usePassThroughHtmlProps'; import { getScrollBarWidth } from '@ui5/webcomponents-react-base/lib/Utils'; -import { AvatarSize } from '@ui5/webcomponents-react/lib/AvatarSize'; -import { Button } from '@ui5/webcomponents-react/lib/Button'; import { ContentDensity } from '@ui5/webcomponents-react/lib/ContentDensity'; +import { FlexBox } from '@ui5/webcomponents-react/lib/FlexBox'; import { ObjectPageMode } from '@ui5/webcomponents-react/lib/ObjectPageMode'; +import { FlexBoxAlignItems } from '@ui5/webcomponents-react/lib/FlexBoxAlignItems'; +import { FlexBoxDirection } from '@ui5/webcomponents-react/lib/FlexBoxDirection'; import debounce from 'lodash.debounce'; import React, { - Children, CSSProperties, FC, forwardRef, ReactElement, - ReactNode, - ReactNodeArray, RefObject, useCallback, useEffect, - useLayoutEffect, useMemo, useRef, useState @@ -31,19 +28,29 @@ import React, { import { createUseStyles, useTheme } from 'react-jss'; import { CommonProps } from '../../interfaces/CommonProps'; import { JSSTheme } from '../../interfaces/JSSTheme'; +import { Button } from '../../webComponents/Button'; import { ObjectPageSubSectionPropTypes } from '../ObjectPageSubSection'; import { CollapsedAvatar } from './CollapsedAvatar'; import styles from './ObjectPage.jss'; import { ObjectPageAnchorButton } from './ObjectPageAnchorButton'; +import { ObjectPageHeader } from './ObjectPageHeader'; +import { ObjectPageScrollBar } from './ObjectPageScrollBar'; +import { + bindScrollEvent, + findSectionIndexById, + getProportionateScrollTop, + removeScrollEvent, + safeGetChildrenArray +} from './ObjectPageUtils'; export interface ObjectPagePropTypes extends CommonProps { title?: string; subTitle?: string; - image?: string | ReactElement; + image?: string | ReactElement; imageShapeCircle?: boolean; - headerActions?: Array>; + headerActions?: Array>; renderHeaderContent?: () => JSX.Element; - children?: ReactNode | ReactNodeArray; + children?: ReactElement | Array>; mode?: ObjectPageMode; selectedSectionId?: string; selectedSubSectionId?: string; @@ -57,21 +64,9 @@ export interface ObjectPagePropTypes extends CommonProps { renderKeyInfos?: () => JSX.Element; } -const useStyles = createUseStyles>(styles, { name: 'ObjectPage' }); +const useStyles = createUseStyles(styles, { name: 'ObjectPage' }); const defaultScrollbarWidth = 12; -const findSectionIndexById = (sections, id) => { - const index = Children.toArray(sections).findIndex( - (objectPageSection: ReactElement) => objectPageSection.props.id === id - ); - if (index === -1) { - return 0; - } - return index; -}; - -const positionRelativeStyle: CSSProperties = { position: 'relative' }; - /** * import { ObjectPage } from '@ui5/webcomponents-react/lib/ObjectPage'; */ @@ -103,9 +98,7 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp const [selectedSectionIndex, setSelectedSectionIndex] = useState(findSectionIndexById(children, selectedSectionId)); const [selectedSubSectionId, setSelectedSubSectionId] = useState(props.selectedSubSectionId); const [expandHeaderActive, setExpandHeaderActive] = useState(false); - const [isMounted, setIsMounted] = useState(false); const [collapsedHeader, setCollapsedHeader] = useState(renderHeaderContentProp === null); - const theme = useTheme(); const objectPage: RefObject = useConsolidatedRef(ref); const fillerDivDomRef: RefObject = useRef(); @@ -123,40 +116,21 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp const stableBarOnScrollRef = useRef(null); const scroller = useConsolidatedRef(scrollerRef); const [scrollbarWidth, setScrollbarWidth] = useState(defaultScrollbarWidth); + const isMounted = useRef(false); const classes = useStyles(); - const setScrollbarHeight = () => { - requestAnimationFrame(() => { - const scrollbarContainerHeight = - contentScrollContainer.current.getBoundingClientRect().height + - topHeader.current.getBoundingClientRect().height; - innerScrollBar.current.style.height = `${scrollbarContainerHeight}px`; - }); - }; - useEffect(() => { let selectedIndex = findSectionIndexById(children, selectedSectionId); - if (selectedIndex === -1) { - selectedIndex = 0; - } - if (selectedSectionIndex !== selectedIndex) { setSelectedSectionIndex(selectedIndex); } }, [selectedSectionId]); - let content = children; - if (mode === ObjectPageMode.IconTabBar) { - content = Children.toArray(children)[selectedSectionIndex]; - } - - const adjustDummyDivHeight = () => { + const adjustDummyDivHeight = useCallback(() => { return new Promise((resolve) => { requestAnimationFrame(() => { if (!objectPage.current) { - // in case componentWillUnmount didn´t fire - observer.current.disconnect(); return; } @@ -181,11 +155,16 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp heightDiff = heightDiff > 0 ? heightDiff : 0; fillerDivDomRef.current.style.height = `${heightDiff}px`; - setScrollbarHeight(); + requestAnimationFrame(() => { + const scrollbarContainerHeight = + contentScrollContainer.current.getBoundingClientRect().height + + topHeader.current.getBoundingClientRect().height; + innerScrollBar.current.style.height = `${scrollbarContainerHeight}px`; + }); resolve(); }); }); - }; + }, [objectPage, contentContainer, fillerDivDomRef, contentScrollContainer, topHeader, innerScrollBar]); const adjustContentContainerHeight = useCallback(() => { if (contentContainer.current && outerContentContainer.current) { @@ -195,15 +174,38 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp // @ts-ignore const observer = useRef(new ResizeObserver(adjustDummyDivHeight)); + // @ts-ignore const outerContainerObserver = useRef(new ResizeObserver(adjustContentContainerHeight)); + const { contentDensity } = useTheme() as JSSTheme; + + const renderHideHeaderButton = () => { + if (!showHideHeaderButton || renderHeaderContentProp === null || noHeader) return null; + + return ( +