Skip to content

Commit 1e1650d

Browse files
Lukas742Lukas742
authored
fix(ObjectPage & DynamicPage): active elements in headerTitle are always interactive and won't expand the header (#1825)
1 parent 0767970 commit 1e1650d

File tree

10 files changed

+119
-60
lines changed

10 files changed

+119
-60
lines changed

‎docs/2-MigrationGuide.stories.mdx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ We streamlined those APIs by adding components used by the `DynamicPage` to the
2626
#### DynamicPage changes
2727

2828
- `title` has been renamed to `headerTitle`.
29+
- `header` has been renamed to `headerContent`.
2930
- **`DynamicPageTitle`:** `subHeading` has been renamed to `subheading`.
3031
- **`DynamicPageHeader`:** `children` are no longer displayed as `flex` items to support other display types like `grid`. To align children you now need to add the container (like `FlexBox`) and CSS yourself.
3132
<br />
@@ -53,8 +54,8 @@ We streamlined those APIs by adding components used by the `DynamicPage` to the
5354
- **`ObjectPageSection`:** `title` and `titleUppercase` has been renamed. Please use `heading` and `headingUppercase` instead.
5455
- **`ObjectPageSubSection`:** `title` has been renamed to `heading`.
5556
- `title` has been renamed to `headerTitle` and is now defining the upper, static, title section of the `ObjectPage`. It expects to receive the `DynamicPageTitle` component.
56-
- `headerContent` has been renamed to `header` and expects now the `DynamicPageHeader` component to be passed.
57-
- `noHeader` has been removed. It is now sufficient not to set `headerTitle` and `header` to achieve the same behavior.
57+
- `headerContent` now expects the `DynamicPageHeader` component to be passed.
58+
- `noHeader` has been removed. It is now sufficient not to set `headerTitle` and `headerContent` to achieve the same behavior.
5859
- `title`, `subTitle`, `headerActions`, `breadcrumbs` and `keyInfos` should now be passed to the corresponding `DynamicPageTitle` props.
5960

6061
Setting the title section of the `ObjectPage`:

‎packages/main/src/components/DynamicPage/__snapshots__/DynamicPage.test.tsx.snap‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ exports[`DynamicPage always show content header 1`] = `
88
<div
99
class="FlexBox-flexBox FlexBox-flexBoxDirectionRow FlexBox-justifyContentStart FlexBox-alignItemsStretch FlexBox-flexWrapNoWrap DynamicPageTitle-container StdExtPadding-Desktop"
1010
data-component-name="DynamicPageTitle"
11-
data-not-clickable="true"
11+
data-not-clickable="false"
1212
>
1313
<div
1414
class="FlexBox-flexBox FlexBox-flexBoxDirectionRow FlexBox-justifyContentSpaceBetween FlexBox-alignItemsStretch FlexBox-flexWrapNoWrap"
@@ -151,14 +151,14 @@ exports[`DynamicPage always show content header 1`] = `
151151
</div>
152152
</div>
153153
<div
154-
class="Toolbar-outerContainer Toolbar-clear"
154+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
155155
>
156156
<div
157157
class="Toolbar-toolbar"
158158
>
159159
<span
160160
class="spacer"
161-
style="flex-grow: 1;"
161+
style="flex-grow: 1; height: 100%; cursor: pointer;"
162162
/>
163163
<div>
164164
<ui5-button
@@ -323,7 +323,7 @@ exports[`DynamicPage hider header button 1`] = `
323323
<div
324324
class="FlexBox-flexBox FlexBox-flexBoxDirectionRow FlexBox-justifyContentStart FlexBox-alignItemsStretch FlexBox-flexWrapNoWrap DynamicPageTitle-container StdExtPadding-Desktop"
325325
data-component-name="DynamicPageTitle"
326-
data-not-clickable="true"
326+
data-not-clickable="false"
327327
>
328328
<div
329329
class="FlexBox-flexBox FlexBox-flexBoxDirectionRow FlexBox-justifyContentStart FlexBox-alignItemsCenter FlexBox-flexWrapNoWrap"
@@ -344,14 +344,14 @@ exports[`DynamicPage hider header button 1`] = `
344344
</div>
345345
</div>
346346
<div
347-
class="Toolbar-outerContainer Toolbar-clear"
347+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
348348
>
349349
<div
350350
class="Toolbar-toolbar"
351351
>
352352
<span
353353
class="spacer"
354-
style="flex-grow: 1;"
354+
style="flex-grow: 1; height: 100%; cursor: pointer;"
355355
/>
356356
<div>
357357
<ui5-button
@@ -837,14 +837,14 @@ exports[`DynamicPage with content 1`] = `
837837
</div>
838838
</div>
839839
<div
840-
class="Toolbar-outerContainer Toolbar-clear"
840+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
841841
>
842842
<div
843843
class="Toolbar-toolbar"
844844
>
845845
<span
846846
class="spacer"
847-
style="flex-grow: 1;"
847+
style="flex-grow: 1; height: 100%; cursor: pointer;"
848848
/>
849849
<div>
850850
<ui5-button
@@ -1808,14 +1808,14 @@ exports[`DynamicPage without content 1`] = `
18081808
</div>
18091809
</div>
18101810
<div
1811-
class="Toolbar-outerContainer Toolbar-clear"
1811+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
18121812
>
18131813
<div
18141814
class="Toolbar-toolbar"
18151815
>
18161816
<span
18171817
class="spacer"
1818-
style="flex-grow: 1;"
1818+
style="flex-grow: 1; height: 100%; cursor: pointer;"
18191819
/>
18201820
<div>
18211821
<ui5-button

‎packages/main/src/components/DynamicPage/index.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ const DynamicPage: FC<DynamicPageProps> = forwardRef((props: DynamicPageProps, r
224224
{headerTitle &&
225225
cloneElement(headerTitle, {
226226
'data-not-clickable':
227-
alwaysShowContentHeader || !headerContent || (!showHideHeaderButton && !headerContentPinnable),
227+
(alwaysShowContentHeader && !headerContentPinnable) ||
228+
!headerContent ||
229+
(!showHideHeaderButton && !headerContentPinnable),
228230
ref: topHeaderRef,
229231
className: headerTitle?.props?.className
230232
? `${responsivePaddingClass} ${headerTitle.props.className}`

‎packages/main/src/components/DynamicPageTitle/ActionsSpacer.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import React, { MouseEventHandler } from 'react';
2+
3+
interface ActionsSpacerProps {
4+
onClick: MouseEventHandler<HTMLElement>;
5+
noHover?: boolean;
6+
}
7+
8+
export const ActionsSpacer = ({ onClick, noHover }: ActionsSpacerProps) => {
9+
return (
10+
<span
11+
style={{ flexGrow: 1, height: '100%', cursor: noHover ? 'auto' : 'pointer' }}
12+
className="spacer"
13+
onClick={onClick}
14+
/>
15+
);
16+
};
17+
18+
// The Toolbar only recognizes spacers with the 'ToolbarSpacer' displayName
19+
ActionsSpacer.displayName = 'ToolbarSpacer';

‎packages/main/src/components/DynamicPageTitle/DynamicPageTitle.jss.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ export const DynamicPageTitleStyles = {
1515
zIndex: 2,
1616
cursor: 'pointer',
1717
'&[data-not-clickable="true"]': {
18-
pointerEvents: 'none',
1918
cursor: 'unset',
2019
'&:hover': {
2120
backgroundColor: ThemingParameters.sapObjectHeader_Background
@@ -60,5 +59,14 @@ export const DynamicPageTitleStyles = {
6059
content: {
6160
display: 'flex',
6261
flexShrink: 1.6
62+
},
63+
toolbar: {
64+
cursor: 'auto',
65+
'&:hover': {
66+
backgroundColor: 'inherit'
67+
},
68+
'&>:first-child': {
69+
height: '100%'
70+
}
6371
}
6472
};

‎packages/main/src/components/DynamicPageTitle/index.tsx‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@ import React, {
2020
ReactNode,
2121
ReactNodeArray,
2222
Ref,
23+
useCallback,
2324
useEffect,
2425
useState
2526
} from 'react';
2627
import { createUseStyles } from 'react-jss';
28+
import { stopPropagation } from '../../internal/stopPropagation';
29+
import { ActionsSpacer } from './ActionsSpacer';
2730
import { DynamicPageTitleStyles } from './DynamicPageTitle.jss';
2831
import { useIsRTL } from '@ui5/webcomponents-react-base/dist/hooks';
2932

@@ -108,7 +111,19 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
108111
containerClasses.put(classes.iEClass);
109112
}
110113
containerClasses.putIfPresent(className);
111-
const passThroughProps = usePassThroughHtmlProps(props, ['onToggleHeaderContentVisibility']);
114+
const passThroughProps = usePassThroughHtmlProps(props, ['onToggleHeaderContentVisibility', 'onClick']);
115+
116+
const onHeaderClick = useCallback(
117+
(e) => {
118+
if (typeof props?.onClick === 'function') {
119+
props.onClick(e);
120+
}
121+
if (typeof onToggleHeaderContentVisibility === 'function' && !props?.['data-not-clickable']) {
122+
onToggleHeaderContentVisibility(e);
123+
}
124+
},
125+
[props?.onClick, onToggleHeaderContentVisibility, props?.['data-not-clickable']]
126+
);
112127

113128
useEffect(() => {
114129
const observer = new ResizeObserver(
@@ -143,13 +158,19 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
143158
ref={dynamicPageTitleRef}
144159
tooltip={tooltip}
145160
data-component-name="DynamicPageTitle"
146-
onClick={onToggleHeaderContentVisibility}
161+
onClick={onHeaderClick}
147162
{...passThroughProps}
148163
>
149164
{(breadcrumbs || (navigationActions && showNavigationInTopArea)) && (
150165
<FlexBox justifyContent={FlexBoxJustifyContent.SpaceBetween}>
151-
<div className={classes.breadcrumbs}>{breadcrumbs}</div>
152-
{showNavigationInTopArea && <FlexBox alignItems={FlexBoxAlignItems.End}>{navigationActions}</FlexBox>}
166+
<div className={classes.breadcrumbs} onClick={stopPropagation}>
167+
{breadcrumbs}
168+
</div>
169+
{showNavigationInTopArea && (
170+
<FlexBox alignItems={FlexBoxAlignItems.End} onClick={stopPropagation}>
171+
{navigationActions}
172+
</FlexBox>
173+
)}
153174
</FlexBox>
154175
)}
155176
<FlexBox alignItems={FlexBoxAlignItems.Center} style={{ flexGrow: 1, width: '100%' }}>
@@ -166,8 +187,14 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
166187
</div>
167188
)}
168189
</FlexBox>
169-
<Toolbar design={ToolbarDesign.Auto} toolbarStyle={ToolbarStyle.Clear}>
170-
<ToolbarSpacer />
190+
<Toolbar
191+
design={ToolbarDesign.Auto}
192+
toolbarStyle={ToolbarStyle.Clear}
193+
active
194+
className={classes.toolbar}
195+
onClick={stopPropagation}
196+
>
197+
<ActionsSpacer onClick={onHeaderClick} noHover={props?.['data-not-clickable']} />
171198
{actions}
172199
{!showNavigationInTopArea && Children.count(actions) > 0 && Children.count(navigationActions) > 0 && (
173200
<ToolbarSeparator />

‎packages/main/src/components/ObjectPage/ObjectPage.jss.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,15 @@ export const styles = {
6262
}
6363
},
6464
headerHoverStyles: {
65-
backgroundColor: `${ThemingParameters.sapTile_Active_Background} !important`,
66-
'& [data-component-name="DynamicPageTitle"]': {
67-
backgroundColor: ThemingParameters.sapTile_Active_Background
65+
'&[data-not-clickable="true"]': {
66+
cursor: 'unset'
67+
},
68+
'&[data-not-clickable="false"]': {
69+
// TODO background color should be sapObjectHeader_Hover_Background (same color as sapTile_Active_Background)
70+
backgroundColor: `${ThemingParameters.sapTile_Active_Background}`,
71+
'& [data-component-name="DynamicPageTitle"]': {
72+
backgroundColor: ThemingParameters.sapTile_Active_Background
73+
}
6874
}
6975
},
7076
header: {
@@ -79,18 +85,7 @@ export const styles = {
7985
paddingLeft: 0,
8086
paddingRight: 0
8187
},
82-
'&:hover': {
83-
// TODO background color should be sapObjectHeader_Hover_Background (same color as sapTile_Active_Background)
84-
backgroundColor: ThemingParameters.sapTile_Active_Background
85-
},
86-
cursor: 'pointer',
87-
'&[data-not-clickable="true"]': {
88-
pointerEvents: 'none',
89-
cursor: 'unset',
90-
'&:hover': {
91-
backgroundColor: ThemingParameters.sapObjectHeader_Background
92-
}
93-
}
88+
cursor: 'pointer'
9489
},
9590
iEClass: {
9691
position: 'fixed',

‎packages/main/src/components/ObjectPage/__snapshots__/ObjectPage.test.tsx.snap‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -823,14 +823,14 @@ exports[`ObjectPage with anchor-bar 1`] = `
823823
</div>
824824
</div>
825825
<div
826-
class="Toolbar-outerContainer Toolbar-clear"
826+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
827827
>
828828
<div
829829
class="Toolbar-toolbar"
830830
>
831831
<span
832832
class="spacer"
833-
style="flex-grow: 1;"
833+
style="flex-grow: 1; height: 100%; cursor: pointer;"
834834
/>
835835
</div>
836836
</div>
@@ -1881,14 +1881,14 @@ exports[`ObjectPage with img 1`] = `
18811881
</div>
18821882
</div>
18831883
<div
1884-
class="Toolbar-outerContainer Toolbar-clear"
1884+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
18851885
>
18861886
<div
18871887
class="Toolbar-toolbar"
18881888
>
18891889
<span
18901890
class="spacer"
1891-
style="flex-grow: 1;"
1891+
style="flex-grow: 1; height: 100%; cursor: auto;"
18921892
/>
18931893
</div>
18941894
</div>
@@ -2287,14 +2287,14 @@ exports[`ObjectPage with img 2`] = `
22872287
</div>
22882288
</div>
22892289
<div
2290-
class="Toolbar-outerContainer Toolbar-clear"
2290+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
22912291
>
22922292
<div
22932293
class="Toolbar-toolbar"
22942294
>
22952295
<span
22962296
class="spacer"
2297-
style="flex-grow: 1;"
2297+
style="flex-grow: 1; height: 100%; cursor: auto;"
22982298
/>
22992299
</div>
23002300
</div>
@@ -2678,14 +2678,14 @@ exports[`ObjectPage with title 1`] = `
26782678
</div>
26792679
</div>
26802680
<div
2681-
class="Toolbar-outerContainer Toolbar-clear"
2681+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
26822682
>
26832683
<div
26842684
class="Toolbar-toolbar"
26852685
>
26862686
<span
26872687
class="spacer"
2688-
style="flex-grow: 1;"
2688+
style="flex-grow: 1; height: 100%; cursor: auto;"
26892689
/>
26902690
</div>
26912691
</div>
@@ -3019,14 +3019,14 @@ exports[`ObjectPage with title, header & footer 1`] = `
30193019
</div>
30203020
</div>
30213021
<div
3022-
class="Toolbar-outerContainer Toolbar-clear"
3022+
class="Toolbar-outerContainer Toolbar-clear Toolbar-active DynamicPageTitle-toolbar"
30233023
>
30243024
<div
30253025
class="Toolbar-toolbar"
30263026
>
30273027
<span
30283028
class="spacer"
3029-
style="flex-grow: 1;"
3029+
style="flex-grow: 1; height: 100%; cursor: auto;"
30303030
/>
30313031
</div>
30323032
</div>

0 commit comments

Comments
 (0)