Skip to content

Commit 16c0453

Browse files
authored
fix(ObjectPage): fire onPinButtonToggle only when required (#7064)
This PR also improves the types of the `ObjectPageAnchorBar` component.
1 parent 2bc8376 commit 16c0453

File tree

2 files changed

+33
-18
lines changed

2 files changed

+33
-18
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,13 +270,15 @@ describe('ObjectPage', () => {
270270
cy.findByText('ObjectPageHeader').should('not.be.visible');
271271

272272
cy.findByTestId('btn').click();
273+
cy.get('@onPinSpy').should('have.callCount', 3);
273274
cy.findByTestId('op').scrollTo(0, 500);
274275
cy.findByText('ObjectPageHeader').should('be.visible');
275-
276+
cy.wait(200);
276277
cy.findByTestId('btn').click();
277278
cy.findByText('ObjectPageHeader').should('not.be.visible');
278279

279280
cy.get('[data-component-name="ObjectPageAnchorBarExpandBtn"]').click();
281+
cy.get('@onPinSpy').should('have.callCount', 4);
280282
cy.findByText('ObjectPageHeader').should('be.visible');
281283

282284
// wait for timeout of expand click
@@ -294,6 +296,7 @@ describe('ObjectPage', () => {
294296
cy.findByTestId('op').scrollTo(0, 500);
295297
cy.wait(500);
296298
cy.findByTestId('btn').click();
299+
cy.get('@onPinSpy').should('have.callCount', 6);
297300
cy.findByText('ObjectPageHeader').should('not.be.visible');
298301

299302
cy.findByTestId('btn').click();

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

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,17 @@ import iconPushPinOff from '@ui5/webcomponents-icons/dist/pushpin-off.js';
44
import iconPushPinOn from '@ui5/webcomponents-icons/dist/pushpin-on.js';
55
import iconArrowDown from '@ui5/webcomponents-icons/dist/slim-arrow-down.js';
66
import iconArrowUp from '@ui5/webcomponents-icons/dist/slim-arrow-up.js';
7-
import { enrichEventWithDetails, useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
7+
import { debounce, enrichEventWithDetails, useI18nBundle, useStylesheet } from '@ui5/webcomponents-react-base';
88
import { clsx } from 'clsx';
9-
import type { CSSProperties } from 'react';
10-
import { forwardRef, useCallback, useEffect, useRef } from 'react';
9+
import type { CSSProperties, Dispatch, MouseEvent, SetStateAction } from 'react';
10+
import { forwardRef, useEffect, useRef } from 'react';
1111
import { COLLAPSE_HEADER, EXPAND_HEADER, PIN_HEADER, UNPIN_HEADER } from '../../i18n/i18n-defaults.js';
1212
import { cssVarVersionInfoPrefix, getUi5TagWithSuffix } from '../../internal/utils.js';
1313
import type { CommonProps } from '../../types/index.js';
14-
import { Button, ToggleButton } from '../../webComponents/index.js';
15-
import type { ButtonDomRef } from '../../webComponents/index.js';
14+
import type { ButtonDomRef } from '../../webComponents/Button/index.js';
15+
import { Button } from '../../webComponents/Button/index.js';
16+
import type { ToggleButtonDomRef, ToggleButtonPropTypes } from '../../webComponents/ToggleButton/index.js';
17+
import { ToggleButton } from '../../webComponents/ToggleButton/index.js';
1618
import { classNames, styleData } from './ObjectPageAnchorBar.module.css.js';
1719

1820
const _buttonBaseMinWidth = `${cssVarVersionInfoPrefix}button_base_min_width`;
@@ -39,15 +41,15 @@ interface ObjectPageAnchorBarPropTypes extends CommonProps {
3941
/**
4042
* Set the header to the state pinned.
4143
*/
42-
setHeaderPinned?: (payload: any) => void;
44+
setHeaderPinned?: Dispatch<SetStateAction<boolean>>;
4345
/**
4446
* Toggles the header content to be hidden or visible .
4547
*/
4648
onToggleHeaderContentVisibility: (e: any) => void;
4749
/**
4850
* Highlight title when hovered.
4951
*/
50-
onHoverToggleButton?: (e: any) => void;
52+
onHoverToggleButton?: (e: MouseEvent<ToggleButtonDomRef>) => void;
5153
/**
5254
* Defines internally used accessibility properties/attributes.
5355
*/
@@ -83,30 +85,40 @@ const ObjectPageAnchorBar = forwardRef<HTMLElement, ObjectPageAnchorBarPropTypes
8385
const shouldRenderHeaderPinnableButton = !hidePinButton && headerContentVisible;
8486
const showBothActions = shouldRenderHeaderPinnableButton;
8587

86-
const onPinHeader = useCallback(
87-
(e) => {
88-
setHeaderPinned(e.target.pressed);
89-
},
90-
[setHeaderPinned]
91-
);
88+
const onPinHeader: ToggleButtonPropTypes['onClick'] = (e) => {
89+
const target = e.target as ToggleButtonDomRef;
90+
setHeaderPinned(target.pressed);
91+
};
92+
93+
// debounced because of StrictMode
94+
const debouncedOnPinButtonToggle = debounce((pinned: boolean) => {
95+
onPinButtonToggle(pinned);
96+
}, 300);
9297

9398
const isInitial = useRef(true);
9499
useEffect(() => {
95-
if (!isInitial.current && typeof onPinButtonToggle === 'function') {
96-
onPinButtonToggle(headerPinned);
100+
if (!isInitial.current && typeof onPinButtonToggle === 'function' && headerPinned != null) {
101+
debouncedOnPinButtonToggle(headerPinned);
97102
}
98103
if (isInitial.current) {
99104
isInitial.current = false;
100105
}
106+
107+
return () => {
108+
debouncedOnPinButtonToggle.cancel();
109+
};
101110
}, [headerPinned]);
102111

103112
useEffect(() => {
104113
const tagName = getUi5TagWithSuffix('ui5-button');
105114
const showHideHeaderBtn = showHideHeaderBtnRef.current;
106115
void customElements.whenDefined(tagName).then(() => {
107116
if (showHideHeaderBtn) {
108-
//@ts-expect-error: for some reason these optional attributes are mandatory which is not expected
109-
showHideHeaderBtn.accessibilityAttributes = { expanded: !!headerContentVisible };
117+
showHideHeaderBtn.accessibilityAttributes = {
118+
expanded: !!headerContentVisible,
119+
hasPopup: undefined,
120+
controls: undefined
121+
};
110122
}
111123
});
112124
}, [!!headerContentVisible]);

0 commit comments

Comments
 (0)