Skip to content

Commit dba642b

Browse files
authored
fix(ObjectPage): slightly improve scroll performance (#4970)
This PR slightly improves the performance of the ObjectPage (a few ms). It also refactors the Object- and DynamicPage to use logical css properties for padding. Fixes #4960
1 parent f204852 commit dba642b

File tree

5 files changed

+26
-29
lines changed

5 files changed

+26
-29
lines changed

packages/main/src/components/DynamicPage/DynamicPage.jss.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const styles = {
4545
},
4646
contentContainer: {
4747
extend: ResponsiveContainerPadding,
48-
paddingTop: '1rem',
48+
paddingBlockStart: '1rem',
4949
boxSizing: 'border-box',
5050
width: '100%',
5151
height: 'auto',

packages/main/src/components/DynamicPageHeader/DynamicPageHeader.jss.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ export const DynamicPageHeaderStyles = {
66
backgroundColor: ThemingParameters.sapObjectHeader_Background,
77
position: 'sticky',
88
zIndex: 1,
9-
paddingTop: '1rem',
10-
paddingBottom: '1rem',
9+
paddingBlockStart: '1rem',
10+
paddingBlockEnd: '1rem',
1111
display: `var(${DynamicPageCssVariables.headerDisplay})`,
1212
overflow: 'hidden'
1313
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ export const DynamicPageTitleStyles = {
66
flex: '1 1 100%',
77
backgroundColor: ThemingParameters.sapObjectHeader_Background,
88
minHeight: '3rem',
9-
paddingTop: '0.5rem',
10-
paddingBottom: '0.5rem',
9+
paddingBlockStart: '0.5rem',
10+
paddingBlockEnd: '0.5rem',
1111
display: 'flex',
1212
flexDirection: 'column',
1313
justifyContent: 'space-between',
@@ -44,7 +44,7 @@ export const DynamicPageTitleStyles = {
4444
fontFamily: ThemingParameters.sapObjectHeader_Title_FontFamily,
4545
color: ThemingParameters.sapObjectHeader_Title_TextColor,
4646
fontSize: `var(${DynamicPageCssVariables.titleFontSize})`,
47-
paddingTop: '0.3125rem',
47+
paddingBlockStart: '0.3125rem',
4848
overflowWrap: 'break-word',
4949
hyphens: 'auto',
5050
'& > *': {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ export const styles = {
8282
zIndex: 2,
8383
'& [data-component-name="DynamicPageTitle"]': {
8484
gridColumn: 2,
85-
paddingLeft: 0,
86-
paddingRight: 0
85+
paddingInline: 0
8786
},
8887
cursor: 'pointer'
8988
},
@@ -136,5 +135,6 @@ export const styles = {
136135
'&::part(content)': {
137136
padding: 0
138137
}
139-
}
138+
},
139+
titleInHeader: { padding: 0 }
140140
};

packages/main/src/components/ObjectPage/index.tsx

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,19 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
555555
threshold: [0]
556556
}
557557
);
558-
// Fallback when scrolling faster than the IntersectionObserver can observe (in most cases faster than 60fps)
558+
559+
sections.forEach((el) => {
560+
observer.observe(el);
561+
});
562+
563+
return () => {
564+
observer.disconnect();
565+
};
566+
}, [children, totalHeaderHeight, setInternalSelectedSectionId, isProgrammaticallyScrolled]);
567+
568+
// Fallback when scrolling faster than the IntersectionObserver can observe (in most cases faster than 60fps)
569+
useEffect(() => {
570+
const sections = objectPageRef.current?.querySelectorAll('section[data-component-name="ObjectPageSection"]');
559571
if (isAfterScroll) {
560572
let currentSection = sections[sections.length - 1];
561573
let currentIndex;
@@ -582,22 +594,7 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
582594
}
583595
setIsAfterScroll(false);
584596
}
585-
586-
sections.forEach((el) => {
587-
observer.observe(el);
588-
});
589-
590-
return () => {
591-
observer.disconnect();
592-
};
593-
}, [
594-
objectPageRef.current,
595-
children,
596-
totalHeaderHeight,
597-
setInternalSelectedSectionId,
598-
isProgrammaticallyScrolled,
599-
isAfterScroll
600-
]);
597+
}, [isAfterScroll]);
601598

602599
const titleHeaderNotClickable =
603600
(alwaysShowContentHeader && !headerContentPinnable) ||
@@ -616,18 +613,18 @@ const ObjectPage = forwardRef<HTMLDivElement, ObjectPagePropTypes>((props, ref)
616613

617614
const renderTitleSection = useCallback(
618615
(inHeader = false) => {
619-
const titleStyles = { ...(inHeader ? { padding: 0 } : {}), ...(headerTitle?.props?.style ?? {}) };
616+
const titleInHeaderClass = inHeader ? classes.titleInHeader : undefined;
620617

621618
if (headerTitle?.props && headerTitle.props?.showSubHeaderRight === undefined) {
622619
return cloneElement(headerTitle, {
623620
showSubHeaderRight: true,
624-
style: titleStyles,
621+
className: clsx(titleInHeaderClass, headerTitle?.props?.className),
625622
'data-not-clickable': titleHeaderNotClickable,
626623
onToggleHeaderContentVisibility: onTitleClick
627624
});
628625
}
629626
return cloneElement(headerTitle, {
630-
style: titleStyles,
627+
className: clsx(titleInHeaderClass, headerTitle?.props?.className),
631628
'data-not-clickable': titleHeaderNotClickable,
632629
onToggleHeaderContentVisibility: onTitleClick
633630
});

0 commit comments

Comments
 (0)