From bb0d9d4a0023f9ecaf058f8b3bccd049621e96bf Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 29 May 2020 12:44:01 +0200 Subject: [PATCH] fix(FilterBar/ObjectPage): add toolbar for making action bars responsive (#546) Closes #533 --- .../src/components/FilterBar/FilterBar.jss.ts | 4 - .../components/FilterBar/FilterBar.test.tsx | 6 +- .../__snapshots__/FilterBar.test.tsx.snap | 575 ++++++++++-------- .../main/src/components/FilterBar/index.tsx | 30 +- .../components/ObjectPage/CollapsedAvatar.tsx | 5 +- .../components/ObjectPage/ObjectPage.jss.ts | 3 +- .../components/ObjectPage/ObjectPage.test.tsx | 9 + .../ObjectPage/ObjectPageAnchorBar.tsx | 2 +- .../ObjectPage/ObjectPageHeader.tsx | 12 +- .../__snapshots__/ObjectPage.test.tsx.snap | 189 ++++-- .../main/src/components/ObjectPage/index.tsx | 12 +- .../ObjectPage/useObserveHeights.ts | 15 +- .../components/Toolbar/OverflowPopover.tsx | 30 +- .../main/src/components/Toolbar/index.tsx | 31 +- 14 files changed, 553 insertions(+), 370 deletions(-) diff --git a/packages/main/src/components/FilterBar/FilterBar.jss.ts b/packages/main/src/components/FilterBar/FilterBar.jss.ts index 9606b8a4c19..549974f0e00 100644 --- a/packages/main/src/components/FilterBar/FilterBar.jss.ts +++ b/packages/main/src/components/FilterBar/FilterBar.jss.ts @@ -18,10 +18,6 @@ const styles = { boxShadow: 'none', flexWrap: 'wrap' }, - vLine: { - borderLeft: '1px solid gray', - paddingLeft: '0.5rem' - }, filterArea: { display: 'flex', flexWrap: 'wrap', diff --git a/packages/main/src/components/FilterBar/FilterBar.test.tsx b/packages/main/src/components/FilterBar/FilterBar.test.tsx index 0258f8c08b2..af22a18d694 100644 --- a/packages/main/src/components/FilterBar/FilterBar.test.tsx +++ b/packages/main/src/components/FilterBar/FilterBar.test.tsx @@ -12,6 +12,7 @@ import { Select } from '@ui5/webcomponents-react/lib/Select'; import { Switch } from '@ui5/webcomponents-react/lib/Switch'; import { VariantManagement } from '@ui5/webcomponents-react/lib/VariantManagement'; import { MultiComboBoxItem } from '@ui5/webcomponents-react/lib/MultiComboBoxItem'; +import { Toolbar } from '@ui5/webcomponents-react/lib/Toolbar'; import { mount } from 'enzyme'; import React from 'react'; import { act } from 'react-dom/test-utils'; @@ -198,7 +199,8 @@ describe('FilterBar', () => { ); - const openFiltersDialogBtn = wrapper.find('.FilterBar-headerRowRight-0-2-7').childAt(3); + const openFiltersDialogBtn = wrapper.find(Toolbar).find(Button).at(5); + act(() => { openFiltersDialogBtn.prop('onClick')(); }); @@ -266,7 +268,7 @@ describe('FilterBar', () => { ); const filterItemsFB = wrapper.find(FilterGroupItem); - const openFiltersDialogBtn = wrapper.find('.FilterBar-headerRowRight-0-2-7').childAt(3); + const openFiltersDialogBtn = wrapper.find(Toolbar).find(Button).at(3); act(() => { openFiltersDialogBtn.prop('onClick')(); }); diff --git a/packages/main/src/components/FilterBar/__snapshots__/FilterBar.test.tsx.snap b/packages/main/src/components/FilterBar/__snapshots__/FilterBar.test.tsx.snap index 9c136e6c121..f51ddde5aa4 100644 --- a/packages/main/src/components/FilterBar/__snapshots__/FilterBar.test.tsx.snap +++ b/packages/main/src/components/FilterBar/__snapshots__/FilterBar.test.tsx.snap @@ -5,17 +5,23 @@ exports[`FilterBar Hide FilterBar 1`] = ` class="FilterBar-outerContainer-0" >
- - Show Filter Bar - + +
+ + Show Filter Bar + +
- - Hide Filter Bar - + +
+ + Hide Filter Bar + +
- - Variant 1 - - - - - Cancel - - +
- Variant 1 - - + + - Variant 2 - - - -
-
- + Cancel + + + + Variant 1 + + + Variant 2 + + + +
+
+
+
+
+ +
+
+ -
-
- - Clear - - - Restore - - - Hide Filter Bar - - - Filters (1337) - - - Go - +
+ + Clear + + + Restore + + + Hide Filter Bar + + + Filters (1337) + + + Go + +
- - Variant 1 - - - - - Cancel - - +
- Variant 1 - - + + - Variant 2 - - - -
-
- + Cancel + + + + Variant 1 + + + Variant 2 + + + +
+
+
+
+
+ +
+
+ -
-
- - Clear - - - Restore - - - Hide Filter Bar - - - Filters (1337) - - - Go - +
+ + Clear + + + Restore + + + Hide Filter Bar + + + Filters (1337) + + + Go + +
- - Variant 1 - - - - - Cancel - - +
- Variant 1 - - + + - Variant 2 - - - -
-
- + Cancel + + + + Variant 1 + + + Variant 2 + + + +
+
+
+
+
+ +
+
+ -
-
- - Clear - - - Restore - - - Hide Filter Bar - - - Filters (1337) - - - Go - +
+ + Clear + + + Restore + + + Hide Filter Bar + + + Filters (1337) + + + Go + +
= forwardRef((props: FilterBarPropTypes, ) : ( <> -
+ {variants} {search && ( -
- {renderSearchWithValue(search, searchValue)} -
+ <> + +
{renderSearchWithValue(search, searchValue)}
+ )} + {useToolbar && ( -
+ <> {showClearOnFB && ( )} - + { + + } {showFilterConfiguration && ( )} -
+ )} -
+ {mountFilters &&
{renderChildren()}
} )} diff --git a/packages/main/src/components/ObjectPage/CollapsedAvatar.tsx b/packages/main/src/components/ObjectPage/CollapsedAvatar.tsx index cfbd556580e..2e7e36e1c5d 100644 --- a/packages/main/src/components/ObjectPage/CollapsedAvatar.tsx +++ b/packages/main/src/components/ObjectPage/CollapsedAvatar.tsx @@ -18,8 +18,9 @@ const styles = { imageContainer: { display: 'inline-block', verticalAlign: 'middle', - maxWidth: '3rem', - maxHeight: '3rem' + maxHeight: '3rem', + width: '3rem', + maxWidth: '3rem' }, image: { width: '100%', diff --git a/packages/main/src/components/ObjectPage/ObjectPage.jss.ts b/packages/main/src/components/ObjectPage/ObjectPage.jss.ts index 9aeb13690a3..32f1a263ac8 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.jss.ts +++ b/packages/main/src/components/ObjectPage/ObjectPage.jss.ts @@ -84,7 +84,7 @@ const styles = { position: 'relative' }, container: { - width: '70%', + flex: '1 1 70%', boxSizing: 'border-box' }, title: { @@ -146,6 +146,7 @@ const styles = { }, headerImage: { + minWidth: '5rem', maxWidth: '5rem', maxHeight: '5rem', display: 'inline-block', diff --git a/packages/main/src/components/ObjectPage/ObjectPage.test.tsx b/packages/main/src/components/ObjectPage/ObjectPage.test.tsx index 5e809116ea5..15ec6739d00 100644 --- a/packages/main/src/components/ObjectPage/ObjectPage.test.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPage.test.tsx @@ -80,8 +80,17 @@ const renderComponentWithSections = () => ( ); +let original; +beforeAll(() => { + original = Element.prototype.scrollTo; + Element.prototype.scrollTo = Element.prototype.scrollTo || jest.fn(); +}); +afterAll(() => { + Element.prototype.scrollTo = original; +}); describe('ObjectPage', () => { + test('With Subsections', () => { const wrapper = mount(renderComponent()); expect(wrapper.render()).toMatchSnapshot(); diff --git a/packages/main/src/components/ObjectPage/ObjectPageAnchorBar.tsx b/packages/main/src/components/ObjectPage/ObjectPageAnchorBar.tsx index 04e7e41fea8..fce343624a1 100644 --- a/packages/main/src/components/ObjectPage/ObjectPageAnchorBar.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPageAnchorBar.tsx @@ -116,7 +116,7 @@ const ObjectPageAnchorBar = forwardRef((props: Props, ref: RefObject { - setHeaderPinned(e.detail.pressed); + setHeaderPinned(e.target.pressed); }, [setHeaderPinned] ); diff --git a/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx b/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx index a812a382412..b46073bcafd 100644 --- a/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx +++ b/packages/main/src/components/ObjectPage/ObjectPageHeader.tsx @@ -4,6 +4,10 @@ import { FlexBoxDirection } from '@ui5/webcomponents-react/lib/FlexBoxDirection' import { Label } from '@ui5/webcomponents-react/lib/Label'; import { Title } from '@ui5/webcomponents-react/lib/Title'; import { TitleLevel } from '@ui5/webcomponents-react/lib/TitleLevel'; +import { Toolbar } from '@ui5/webcomponents-react/lib/Toolbar'; +import { ToolbarDesign } from '@ui5/webcomponents-react/lib/ToolbarDesign'; +import { ToolbarSpacer } from '@ui5/webcomponents-react/lib/ToolbarSpacer'; +import { ToolbarStyle } from '@ui5/webcomponents-react/lib/ToolbarStyle'; import React, { CSSProperties, forwardRef, ReactElement, ReactNode, RefObject, useMemo } from 'react'; import { safeGetChildrenArray } from './ObjectPageUtils'; @@ -19,6 +23,7 @@ interface Props { subTitle: string; headerPinned: boolean; topHeaderHeight: number; + headerActions: ReactElement[]; } export const ObjectPageHeader = forwardRef((props: Props, ref: RefObject) => { @@ -33,7 +38,8 @@ export const ObjectPageHeader = forwardRef((props: Props, ref: RefObject { @@ -109,6 +115,10 @@ export const ObjectPageHeader = forwardRef((props: Props, ref: RefObject{keyInfos}
+ + + {headerActions} + ); diff --git a/packages/main/src/components/ObjectPage/__snapshots__/ObjectPage.test.tsx.snap b/packages/main/src/components/ObjectPage/__snapshots__/ObjectPage.test.tsx.snap index 5ba77422341..23ccf0eb827 100644 --- a/packages/main/src/components/ObjectPage/__snapshots__/ObjectPage.test.tsx.snap +++ b/packages/main/src/components/ObjectPage/__snapshots__/ObjectPage.test.tsx.snap @@ -11,15 +11,6 @@ exports[`ObjectPage IconTabBar Mode 1`] = ` role="banner" style="padding-right: 12px;" > - - - Action - -
@@ -48,6 +39,25 @@ exports[`ObjectPage IconTabBar Mode 1`] = ` />
+
+
+ +
+ + Action + +
+
+
@@ -187,9 +197,6 @@ exports[`ObjectPage Just Some Sections 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -214,6 +221,18 @@ exports[`ObjectPage Just Some Sections 1`] = ` />
+
+
+ +
+
@@ -295,9 +314,6 @@ exports[`ObjectPage Key Infos 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -387,6 +403,18 @@ exports[`ObjectPage Key Infos 1`] = ` +
+
+ +
+
@@ -491,9 +519,6 @@ exports[`ObjectPage No Header 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -518,6 +543,18 @@ exports[`ObjectPage No Header 1`] = ` /> +
+
+ +
+
@@ -622,9 +659,6 @@ exports[`ObjectPage Not crashing with 0 sections 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -649,6 +683,18 @@ exports[`ObjectPage Not crashing with 0 sections 1`] = ` /> +
+
+ +
+
@@ -693,9 +739,6 @@ exports[`ObjectPage Not crashing with 1 section - Default Mode 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -720,6 +763,18 @@ exports[`ObjectPage Not crashing with 1 section - Default Mode 1`] = ` /> +
+
+ +
+
@@ -795,9 +850,6 @@ exports[`ObjectPage Not crashing with 1 section - IconTabBar Mode 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -822,6 +874,18 @@ exports[`ObjectPage Not crashing with 1 section - IconTabBar Mode 1`] = ` /> +
+
+ +
+
@@ -897,15 +961,6 @@ exports[`ObjectPage Only Sections 1`] = ` role="banner" style="padding-right: 12px;" > - - - Action - -
@@ -934,6 +989,25 @@ exports[`ObjectPage Only Sections 1`] = ` /> +
+
+ +
+ + Action + +
+
+
@@ -1109,9 +1183,6 @@ exports[`ObjectPage Set selected section id 1`] = ` role="banner" style="padding-right: 12px;" > -
@@ -1136,6 +1207,18 @@ exports[`ObjectPage Set selected section id 1`] = ` /> +
+
+ +
+
@@ -1217,15 +1300,6 @@ exports[`ObjectPage With Subsections 1`] = ` role="banner" style="padding-right: 12px;" > - - - Action - -
@@ -1254,6 +1328,25 @@ exports[`ObjectPage With Subsections 1`] = ` /> +
+
+ +
+ + Action + +
+
+
diff --git a/packages/main/src/components/ObjectPage/index.tsx b/packages/main/src/components/ObjectPage/index.tsx index 3f68b99ca6c..6fea65f7666 100644 --- a/packages/main/src/components/ObjectPage/index.tsx +++ b/packages/main/src/components/ObjectPage/index.tsx @@ -11,6 +11,11 @@ import { Label } from '@ui5/webcomponents-react/lib/Label'; import { ObjectPageMode } from '@ui5/webcomponents-react/lib/ObjectPageMode'; import { Title } from '@ui5/webcomponents-react/lib/Title'; import { TitleLevel } from '@ui5/webcomponents-react/lib/TitleLevel'; +import { Toolbar } from '@ui5/webcomponents-react/lib/Toolbar'; +import { ToolbarDesign } from '@ui5/webcomponents-react/lib/ToolbarDesign'; +import { ToolbarSpacer } from '@ui5/webcomponents-react/lib/ToolbarSpacer'; +import { ToolbarStyle } from '@ui5/webcomponents-react/lib/ToolbarStyle'; +import { CommonProps } from '@ui5/webcomponents-react/interfaces/CommonProps'; import debounce from 'lodash.debounce'; import React, { ComponentType, @@ -25,7 +30,6 @@ import React, { useRef, useState } from 'react'; -import { CommonProps } from '../../interfaces/CommonProps'; import { ObjectPageSectionPropTypes } from '../ObjectPageSection'; import { ObjectPageSubSectionPropTypes } from '../ObjectPageSubSection'; import { CollapsedAvatar } from './CollapsedAvatar'; @@ -420,7 +424,6 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp style={scrollBarWidthPadding} className={classes.header} > - {headerActions}
{(!showTitleInHeaderContent || headerContentHeight === 0) && ( @@ -439,11 +442,16 @@ const ObjectPage: FC = forwardRef((props: ObjectPagePropTyp
{keyInfos}
 + + + {headerActions} + )}
{ const [topHeaderHeight, setTopHeaderHeight] = useState(0); const [headerContentHeight, setHeaderContentHeight] = useState(0); + const [isIntersecting, setIsIntersecting] = useState(true); useEffect(() => { const headerIntersectionObserver = new IntersectionObserver( ([header]) => { if (header.isIntersecting) { + setIsIntersecting(true); setHeaderContentHeight((header.target as HTMLElement).offsetHeight); } else { + setIsIntersecting(false); setHeaderContentHeight(0); } }, - { rootMargin: `-${topHeaderHeight}px 0px 0px 0px`, root: objectPage.current, threshold: 0.5 } + { rootMargin: `-${topHeaderHeight}px 0px 0px 0px`, root: objectPage.current, threshold: 0.3 } ); if (headerContentRef.current) { @@ -25,7 +28,7 @@ export const useObserveHeights = (objectPage, topHeader, headerContentRef, ancho return () => { headerIntersectionObserver.disconnect(); }; - }, [topHeaderHeight, setHeaderContentHeight, headerContentRef]); + }, [topHeaderHeight, setHeaderContentHeight, headerContentRef, setIsIntersecting]); // top header useEffect(() => { @@ -38,12 +41,14 @@ export const useObserveHeights = (objectPage, topHeader, headerContentRef, ancho return () => { headerContentResizeObserver.disconnect(); }; - }, [topHeader.current, setHeaderContentHeight]); + }, [topHeader.current, setTopHeaderHeight]); // header content useEffect(() => { const headerContentResizeObserver = new ResizeObserver(([headerContent]) => { - setHeaderContentHeight(headerContent?.contentRect?.height ?? 0); + if (isIntersecting) { + setHeaderContentHeight(headerContent?.contentRect?.height ?? 0); + } }); if (headerContentRef.current) { @@ -52,7 +57,7 @@ export const useObserveHeights = (objectPage, topHeader, headerContentRef, ancho return () => { headerContentResizeObserver.disconnect(); }; - }, [headerContentRef.current, setHeaderContentHeight]); + }, [headerContentRef.current, setHeaderContentHeight, isIntersecting]); const anchorBarHeight = anchorBarRef.current?.offsetHeight ?? 33; const totalHeaderHeight = (noHeader ? 0 : topHeaderHeight + headerContentHeight) + anchorBarHeight; diff --git a/packages/main/src/components/Toolbar/OverflowPopover.tsx b/packages/main/src/components/Toolbar/OverflowPopover.tsx index 5a7d6a3e23f..5de8a3090d3 100644 --- a/packages/main/src/components/Toolbar/OverflowPopover.tsx +++ b/packages/main/src/components/Toolbar/OverflowPopover.tsx @@ -36,22 +36,24 @@ export function OverflowPopover(props) { }; const renderChildren = useCallback(() => { - return React.Children.toArray(children).map((item: ReactElement, index) => { - if (index > lastVisibleIndex) { - if (item.type.displayName === 'ToolbarSeparator') { - return React.cloneElement(item, { - style: { - height: '0.0625rem', - margin: '0.375rem 0.1875rem', - width: '100%', - background: ThemingParameters.sapToolbar_SeparatorColor - } - }); + return React.Children.toArray(children?.type === React.Fragment ? children.props.children : children).map( + (item: ReactElement, index) => { + if (index > lastVisibleIndex) { + if (item.type.displayName === 'ToolbarSeparator') { + return React.cloneElement(item, { + style: { + height: '0.0625rem', + margin: '0.375rem 0.1875rem', + width: '100%', + background: ThemingParameters.sapToolbar_SeparatorColor + } + }); + } + return item; } - return item; + return null; } - return null; - }); + ); }, [children, lastVisibleIndex]); return ( diff --git a/packages/main/src/components/Toolbar/index.tsx b/packages/main/src/components/Toolbar/index.tsx index a4986b83fd5..ffb505e3b8c 100644 --- a/packages/main/src/components/Toolbar/index.tsx +++ b/packages/main/src/components/Toolbar/index.tsx @@ -72,21 +72,24 @@ const Toolbar: FC = forwardRef((props: ToolbarProptypes, ref: const childrenWithRef = useMemo(() => { controlMetaData.current = []; - return React.Children.toArray(children).map((item, index) => { - const itemRef: RefObject = createRef(); - - controlMetaData.current.push({ - ref: itemRef - }); - if (item?.type?.displayName === 'ToolbarSpacer') { - return item; + return React.Children.toArray(children?.type === React.Fragment ? children.props.children : children).map( + (item, index) => { + const itemRef: RefObject = createRef(); + + controlMetaData.current.push({ + ref: itemRef + }); + + if (item?.type?.displayName === 'ToolbarSpacer') { + return item; + } + return ( +
+ {item} +
+ ); } - return ( -
- {item} -
- ); - }); + ); }, [children, controlMetaData]); const overflowNeeded = (lastVisibleIndex || lastVisibleIndex === 0) && React.Children.count(childrenWithRef) !== lastVisibleIndex + 1;