Skip to content

Commit ad9937a

Browse files
authored
fix(ObjectPage): scroll to section when programmatically selected (#6768)
Fixes #6765
1 parent 7301261 commit ad9937a

File tree

3 files changed

+109
-56
lines changed

3 files changed

+109
-56
lines changed

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

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,64 @@ describe('ObjectPage', () => {
10561056
cy.findByText('First Content').should('be.visible');
10571057
});
10581058

1059+
it('programmatic prop selection', () => {
1060+
const TestComp = (props: ObjectPagePropTypes) => {
1061+
const [selectedSection, setSelectedSection] = useState(props.selectedSectionId);
1062+
const [selectedSubSection, setSelectedSubSection] = useState(props.selectedSubSectionId);
1063+
1064+
return (
1065+
<>
1066+
<button
1067+
onClick={() => {
1068+
setSelectedSection('goals');
1069+
}}
1070+
>
1071+
Select Goals
1072+
</button>
1073+
<button
1074+
onClick={() => {
1075+
setSelectedSubSection('personal-payment-information');
1076+
}}
1077+
>
1078+
Select Payment Information
1079+
</button>
1080+
<ObjectPage
1081+
{...props}
1082+
selectedSubSectionId={selectedSubSection}
1083+
selectedSectionId={selectedSection}
1084+
style={{ height: '1000px', scrollBehavior: 'auto' }}
1085+
>
1086+
{OPContent}
1087+
</ObjectPage>
1088+
</>
1089+
);
1090+
};
1091+
1092+
[{ titleArea: DPTitle, headerArea: DPContent }, { titleArea: DPTitle }, { headerArea: DPContent }, {}].forEach(
1093+
(props: ObjectPagePropTypes) => {
1094+
cy.mount(<TestComp {...props} selectedSubSectionId={`employment-job-relationship`} />);
1095+
cy.findByText('employment-job-relationship-content').should('be.visible');
1096+
cy.findByText('Job Information').should('not.be.visible');
1097+
cy.get('[ui5-tabcontainer]').findUi5TabByText('Employment').should('have.attr', 'aria-selected', 'true');
1098+
1099+
cy.mount(<TestComp {...props} selectedSectionId={`personal`} />);
1100+
cy.findByText('personal-connect-content').should('be.visible');
1101+
cy.findByText('test-content').should('not.be.visible');
1102+
cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true');
1103+
1104+
cy.findByText('Select Goals').click();
1105+
cy.findByText('goals-content').should('be.visible');
1106+
cy.findByText('personal-connect-content').should('not.be.visible');
1107+
cy.get('[ui5-tabcontainer]').findUi5TabByText('Goals').should('have.attr', 'aria-selected', 'true');
1108+
1109+
cy.findByText('Select Payment Information').click();
1110+
cy.findByText('personal-payment-information-content').should('be.visible');
1111+
cy.findByText('personal-connect-content').should('not.be.visible');
1112+
cy.get('[ui5-tabcontainer]').findUi5TabByText('Personal').should('have.attr', 'aria-selected', 'true');
1113+
}
1114+
);
1115+
});
1116+
10591117
cypressPassThroughTestsFactory(ObjectPage);
10601118
});
10611119

@@ -1073,7 +1131,7 @@ const DPTitle = (
10731131
<Breadcrumbs>
10741132
<BreadcrumbsItem>Manager Cockpit</BreadcrumbsItem>
10751133
<BreadcrumbsItem>My Team</BreadcrumbsItem>
1076-
<BreadcrumbsItem>Employee Details</BreadcrumbsItem>
1134+
<BreadcrumbsItem>Employee Details (Denise Smith)</BreadcrumbsItem>
10771135
</Breadcrumbs>
10781136
}
10791137
>
@@ -1101,10 +1159,14 @@ const DPContent = (
11011159

11021160
const OPContent = [
11031161
<ObjectPageSection key="0" titleText="Goals" id="goals" aria-label="Goals">
1104-
<div data-testid="section 1" style={{ height: '400px', width: '100%', background: 'lightblue' }} />
1162+
<div data-testid="section 1" style={{ height: '400px', width: '100%', background: 'lightblue' }}>
1163+
<span data-testid="goals-content">goals-content</span>
1164+
</div>
11051165
</ObjectPageSection>,
11061166
<ObjectPageSection key="1" titleText="Test" id="test" aria-label="Test">
1107-
<div data-testid="section 2" style={{ height: '1200px', width: '100%', background: 'lightyellow' }}></div>
1167+
<div data-testid="section 2" style={{ height: '1200px', width: '100%', background: 'lightyellow' }}>
1168+
<span data-testid="test-content">test-content</span>
1169+
</div>
11081170
</ObjectPageSection>,
11091171
<ObjectPageSection key="2" titleText="Personal" id="personal" aria-label="Personal">
11101172
<ObjectPageSubSection
@@ -1121,25 +1183,35 @@ const OPContent = [
11211183
</>
11221184
}
11231185
>
1124-
<div style={{ height: '400px', width: '100%', background: 'black' }} />
1186+
<div style={{ height: '400px', width: '100%', background: 'black' }}>
1187+
<span data-testid="personal-connect-content">personal-connect-content</span>
1188+
</div>
11251189
</ObjectPageSubSection>
11261190
<ObjectPageSubSection
11271191
titleText="Payment Information"
11281192
id="personal-payment-information"
11291193
aria-label="Payment Information"
11301194
>
1131-
<div style={{ height: '400px', width: '100%', background: 'blue' }} />
1195+
<div style={{ height: '400px', width: '100%', background: 'blue' }}>
1196+
<span data-testid="personal-payment-information-content">personal-payment-information-content</span>
1197+
</div>
11321198
</ObjectPageSubSection>
11331199
</ObjectPageSection>,
11341200
<ObjectPageSection key="3" titleText="Employment" id={`~\`!1@#$%^&*()-_+={}[]:;"'z,<.>/?|♥`} aria-label="Employment">
11351201
<ObjectPageSubSection titleText="Job Information" id="employment-job-information" aria-label="Job Information">
1136-
<div style={{ height: '100px', width: '100%', background: 'orange' }}></div>
1202+
<div style={{ height: '100px', width: '100%', background: 'orange' }}>
1203+
<span data-testid="employment-job-information-content">employment-job-information-content</span>
1204+
</div>
11371205
</ObjectPageSubSection>
11381206
<ObjectPageSubSection titleText="Employee Details" id="employment-employee-details" aria-label="Employee Details">
1139-
<div style={{ height: '100px', width: '100%', background: 'cadetblue' }}></div>
1207+
<div style={{ height: '100px', width: '100%', background: 'cadetblue' }}>
1208+
<span data-testid="employment-employee-details-content">employment-employee-details-content</span>
1209+
</div>
11401210
</ObjectPageSubSection>
11411211
<ObjectPageSubSection titleText="Job Relationship" id="employment-job-relationship" aria-label="Job Relationship">
1142-
<div style={{ height: '100px', width: '100%', background: 'lightgrey' }}></div>
1212+
<div style={{ height: '100px', width: '100%', background: 'lightgrey' }}>
1213+
<span data-testid="employment-job-relationship-content">employment-job-relationship-content</span>
1214+
</div>
11431215
</ObjectPageSubSection>
11441216
</ObjectPageSection>
11451217
];

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ export const getSectionById = (sections, id) => {
1111
);
1212
});
1313
};
14+
15+
export const getSectionElementById = (objectPage: HTMLDivElement, isSubSection: boolean, id: string | undefined) => {
16+
return objectPage?.querySelector<HTMLElement>(
17+
`#${isSubSection ? 'ObjectPageSubSection' : 'ObjectPageSection'}-${CSS.escape(id)}`
18+
);
19+
};

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

Lines changed: 23 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import type {
3131
} from '../ObjectPageTitle/index.js';
3232
import { CollapsedAvatar } from './CollapsedAvatar.js';
3333
import { classNames, styleData } from './ObjectPage.module.css.js';
34-
import { getSectionById } from './ObjectPageUtils.js';
34+
import { getSectionById, getSectionElementById } from './ObjectPageUtils.js';
3535

3636
const ObjectPageCssVariables = {
3737
headerDisplay: '--_ui5wcr_ObjectPage_header_display',
@@ -218,10 +218,10 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
218218
const [internalSelectedSectionId, setInternalSelectedSectionId] = useState<string | undefined>(
219219
selectedSectionId ?? firstSectionId
220220
);
221-
const [selectedSubSectionId, setSelectedSubSectionId] = useState(props.selectedSubSectionId);
221+
const [selectedSubSectionId, setSelectedSubSectionId] = useState(undefined);
222222
const [headerPinned, setHeaderPinned] = useState(headerPinnedProp);
223223
const isProgrammaticallyScrolled = useRef(false);
224-
const prevSelectedSectionId = useRef<string | undefined>(undefined);
224+
const [isMounted, setIsMounted] = useState(false);
225225

226226
const [componentRef, objectPageRef] = useSyncRef(ref);
227227
const topHeaderRef = useRef<HTMLDivElement>(null);
@@ -326,9 +326,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
326326
}, [image, classNames.headerImage, classNames.image, imageShapeCircle]);
327327

328328
const scrollToSectionById = (id: string | undefined, isSubSection = false) => {
329-
const section = objectPageRef.current?.querySelector<HTMLElement>(
330-
`#${isSubSection ? 'ObjectPageSubSection' : 'ObjectPageSection'}-${CSS.escape(id)}`
331-
);
329+
const section = getSectionElementById(objectPageRef.current, isSubSection, id);
332330
scrollTimeout.current = performance.now() + 500;
333331
if (section) {
334332
const safeTopHeaderHeight = topHeaderHeight || prevTopHeaderHeight.current;
@@ -367,45 +365,6 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
367365
isProgrammaticallyScrolled.current = false;
368366
};
369367

370-
const programmaticallySetSection = () => {
371-
const currentId = selectedSectionId ?? firstSectionId;
372-
if (currentId !== prevSelectedSectionId.current) {
373-
debouncedOnSectionChange.cancel();
374-
isProgrammaticallyScrolled.current = true;
375-
setInternalSelectedSectionId(currentId);
376-
prevSelectedSectionId.current = currentId;
377-
const sectionNodes = objectPageRef.current?.querySelectorAll('section[data-component-name="ObjectPageSection"]');
378-
const currentIndex = childrenArray.findIndex((objectPageSection) => {
379-
return isValidElement(objectPageSection) && objectPageSection.props?.id === currentId;
380-
});
381-
fireOnSelectedChangedEvent({}, currentIndex, currentId, sectionNodes[0]);
382-
}
383-
};
384-
385-
// change selected section when prop is changed (external change)
386-
const [timeStamp, setTimeStamp] = useState(0);
387-
const requestAnimationFrameRef = useRef<undefined | number>(undefined);
388-
useEffect(() => {
389-
if (selectedSectionId) {
390-
if (mode === ObjectPageMode.Default) {
391-
// wait for DOM draw, otherwise initial scroll won't work as intended
392-
if (timeStamp < 750 && timeStamp !== undefined) {
393-
requestAnimationFrameRef.current = requestAnimationFrame((internalTimestamp) => {
394-
setTimeStamp(internalTimestamp);
395-
});
396-
} else {
397-
setTimeStamp(undefined);
398-
programmaticallySetSection();
399-
}
400-
} else {
401-
programmaticallySetSection();
402-
}
403-
}
404-
return () => {
405-
cancelAnimationFrame(requestAnimationFrameRef.current);
406-
};
407-
}, [timeStamp, selectedSectionId, firstSectionId, debouncedOnSectionChange]);
408-
409368
// section was selected by clicking on the tab bar buttons
410369
const handleOnSectionSelected = (targetEvent, newSelectionSectionId, index, section) => {
411370
isProgrammaticallyScrolled.current = true;
@@ -421,12 +380,24 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
421380
fireOnSelectedChangedEvent(targetEvent, index, newSelectionSectionId, section);
422381
};
423382

383+
useEffect(() => {
384+
if (selectedSectionId) {
385+
const selectedSection = getSectionElementById(objectPageRef.current, false, selectedSectionId);
386+
if (selectedSection) {
387+
const selectedSectionIndex = Array.from(
388+
selectedSection.parentElement.querySelectorAll(':scope > [data-component-name="ObjectPageSection"]')
389+
).indexOf(selectedSection);
390+
handleOnSectionSelected({}, selectedSectionId, selectedSectionIndex, selectedSection);
391+
}
392+
}
393+
}, [selectedSectionId]);
394+
424395
// do internal scrolling
425396
useEffect(() => {
426397
if (mode === ObjectPageMode.Default && isProgrammaticallyScrolled.current === true && !selectedSubSectionId) {
427398
scrollToSection(internalSelectedSectionId);
428399
}
429-
}, [internalSelectedSectionId, mode, isProgrammaticallyScrolled, scrollToSection, selectedSubSectionId]);
400+
}, [internalSelectedSectionId, mode, selectedSubSectionId]);
430401

431402
// Scrolling for Sub Section Selection
432403
useEffect(() => {
@@ -457,11 +428,15 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
457428
}, [headerPinned, topHeaderHeight]);
458429

459430
useEffect(() => {
431+
if (!isMounted) {
432+
setIsMounted(true);
433+
return;
434+
}
460435
setSelectedSubSectionId(props.selectedSubSectionId);
461436
if (props.selectedSubSectionId) {
462437
isProgrammaticallyScrolled.current = true;
463438
if (mode === ObjectPageMode.IconTabBar) {
464-
let sectionId;
439+
let sectionId: string;
465440
childrenArray.forEach((section) => {
466441
if (isValidElement(section) && section.props && section.props.children) {
467442
safeGetChildrenArray(section.props.children).forEach((subSection) => {
@@ -480,7 +455,7 @@ const ObjectPage = forwardRef<ObjectPageDomRef, ObjectPagePropTypes>((props, ref
480455
}
481456
}
482457
}
483-
}, [props.selectedSubSectionId, childrenArray, mode]);
458+
}, [props.selectedSubSectionId, isMounted]);
484459

485460
const tabContainerContainerRef = useRef(null);
486461
useEffect(() => {

0 commit comments

Comments
 (0)