Skip to content

Commit 86d8596

Browse files
Lukas742Lukas742
committed
fix(Toolbar): exclude spacer from overflow popover
Fixes #6933 part cherry-pick of #6962
1 parent d7b03b7 commit 86d8596

File tree

3 files changed

+48
-48
lines changed

3 files changed

+48
-48
lines changed

‎packages/main/src/components/Toolbar/OverflowPopover.tsx

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ interface OverflowPopoverProps {
2525
children: ReactNode[];
2626
portalContainer: Element;
2727
overflowContentRef: Ref<HTMLDivElement>;
28-
numberOfAlwaysVisibleItems?: number;
2928
overflowPopoverRef?: Ref<PopoverDomRef>;
3029
overflowButton?: ReactElement<ToggleButtonPropTypes> | ReactElement<ButtonPropTypes>;
3130
setIsMounted: Dispatch<SetStateAction<boolean>>;
@@ -41,7 +40,6 @@ export const OverflowPopover: FC<OverflowPopoverProps> = (props: OverflowPopover
4140
children,
4241
portalContainer,
4342
overflowContentRef,
44-
numberOfAlwaysVisibleItems,
4543
overflowButton,
4644
overflowPopoverRef,
4745
setIsMounted,
@@ -118,43 +116,45 @@ export const OverflowPopover: FC<OverflowPopoverProps> = (props: OverflowPopover
118116
})();
119117

120118
const OverflowPopoverContextProvider = getOverflowPopoverContext().Provider;
119+
const visibleChildren = lastVisibleIndex === -1 ? children : children.slice(lastVisibleIndex + 1);
121120

122-
let startIndex = null;
123-
const filteredChildrenArray = children
121+
const filteredChildren = (visibleChildren as ReactElement[])
122+
// @ts-expect-error: if type is not defined, it's not a spacer
123+
.filter((child) => child.type?.displayName !== 'ToolbarSpacer' && isValidElement(child))
124124
.map((item, index, arr) => {
125-
if (index > lastVisibleIndex && index > numberOfAlwaysVisibleItems - 1 && isValidElement(item)) {
126-
if (startIndex === null) {
127-
startIndex = index;
128-
}
129-
const labelProp = item?.props?.['data-accessible-name'] ? 'accessibleName' : 'aria-label';
130-
let labelVal = i18nBundle.getText(X_OF_Y, index + 1 - startIndex, arr.length - startIndex);
131-
if (item?.props?.[labelProp]) {
132-
labelVal += ' ' + item.props[labelProp];
133-
}
134-
if (item?.props?.id) {
135-
return cloneElement<HTMLAttributes<HTMLElement>>(item, {
136-
id: `${item.props.id}-overflow`,
137-
[labelProp]: labelVal
138-
});
139-
}
140-
// @ts-expect-error: if type is not defined, it's not a spacer
141-
if (item.type?.displayName === 'ToolbarSeparator') {
142-
return cloneElement(item as ReactElement, {
143-
style: {
144-
height: '0.0625rem',
145-
margin: '0.375rem 0.1875rem',
146-
width: '100%'
147-
},
148-
'aria-label': labelVal
149-
});
150-
}
125+
const labelProp = item?.props?.['data-accessible-name'] ? 'accessibleName' : 'aria-label';
126+
let labelVal = i18nBundle.getText(X_OF_Y, index + 1, arr.length);
127+
if (item?.props?.[labelProp]) {
128+
labelVal += ' ' + item.props[labelProp];
129+
}
130+
131+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
132+
// @ts-ignore: React 19
133+
if (item?.props?.id) {
151134
return cloneElement<HTMLAttributes<HTMLElement>>(item, {
135+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
136+
// @ts-ignore: React 19
137+
id: `${item.props.id}-overflow`,
152138
[labelProp]: labelVal
153139
});
154140
}
155-
return null;
156-
})
157-
.filter(Boolean);
141+
// @ts-expect-error: if type is not defined, it's not a separator
142+
if (item.type?.displayName === 'ToolbarSeparator') {
143+
return cloneElement(item, {
144+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
145+
// @ts-ignore: React 19
146+
style: {
147+
height: '0.0625rem',
148+
margin: '0.375rem 0.1875rem',
149+
width: '100%'
150+
},
151+
'aria-label': labelVal
152+
});
153+
}
154+
return cloneElement<HTMLAttributes<HTMLElement>>(item, {
155+
[labelProp]: labelVal
156+
});
157+
});
158158

159159
return (
160160
<OverflowPopoverContextProvider value={{ inPopover: true }}>
@@ -185,15 +185,15 @@ export const OverflowPopover: FC<OverflowPopoverProps> = (props: OverflowPopover
185185
onAfterOpen={handleAfterOpen}
186186
hideArrow
187187
accessibleRole={accessibleRole}
188-
accessibleName={i18nBundle.getText(WITH_X_ITEMS, filteredChildrenArray.length)}
188+
accessibleName={i18nBundle.getText(WITH_X_ITEMS, filteredChildren.length)}
189189
>
190190
<div
191191
className={classes.popoverContent}
192192
ref={overflowContentRef}
193193
role={a11yConfig?.overflowPopover?.contentRole}
194194
data-component-name="ToolbarOverflowPopoverContent"
195195
>
196-
{filteredChildrenArray}
196+
{filteredChildren}
197197
</div>
198198
</Popover>,
199199
portalContainer ?? document.body

‎packages/main/src/components/Toolbar/Toolbar.cy.tsx

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ const OverflowTestComponent = (props: PropTypes) => {
7979
<Text data-testid="toolbar-item" style={{ width: '200px' }}>
8080
Item1
8181
</Text>
82+
{/*not rendered in overflow popover*/}
8283
<ToolbarSpacer data-testid="spacer1" />
8384
<Text data-testid="toolbar-item2" style={{ width: '200px' }}>
8485
Item2
@@ -143,7 +144,7 @@ describe('Toolbar', () => {
143144
cy.mount(<OverflowTestComponent onOverflowChange={onOverflowChange} />);
144145
cy.get('@overflowChangeSpy').should('have.been.calledOnce');
145146
cy.findByTestId('toolbarElements').should('have.text', 2);
146-
cy.findByTestId('overflowElements').should('have.text', 4);
147+
cy.findByTestId('overflowElements').should('have.text', 3);
147148
cy.findByText('Item1').should('be.visible');
148149
cy.get('[data-testid="toolbar-item2"]').should('not.be.visible');
149150
cy.get('[data-testid="toolbar-item3"]').should('not.be.visible');
@@ -156,22 +157,22 @@ describe('Toolbar', () => {
156157

157158
cy.viewport(500, 500);
158159

159-
// fuzzy - remount component instead
160+
// flaky - remount component instead
160161
// cy.get(`[ui5-toggle-button]`).click();
161162
cy.mount(<OverflowTestComponent onOverflowChange={onOverflowChange} />);
162163
cy.get('[ui5-popover]').should('not.have.attr', 'open');
163164

164165
cy.get('@overflowChangeSpy').should('have.callCount', 2);
165166
cy.findByTestId('toolbarElements').should('have.text', 3);
166-
cy.findByTestId('overflowElements').should('have.text', 3);
167+
cy.findByTestId('overflowElements').should('have.text', 2);
167168

168169
cy.findByTestId('input').shadow().find('input').type('100');
169170
cy.findByTestId('input').trigger('change');
170171
cy.findByTestId('input').shadow().find('input').clear({ force: true });
171172

172173
cy.get('@overflowChangeSpy').should('have.callCount', 3);
173174
cy.findByTestId('toolbarElements').should('have.text', 0);
174-
cy.findByTestId('overflowElements').should('have.text', 6);
175+
cy.findByTestId('overflowElements').should('have.text', 4);
175176

176177
cy.get('[data-testid="toolbar-item"]').should('not.be.visible');
177178
cy.get('[data-testid="toolbar-item2"]').should('not.be.visible');
@@ -193,13 +194,13 @@ describe('Toolbar', () => {
193194

194195
cy.get('@overflowChangeSpy').should('have.callCount', 5);
195196
cy.findByTestId('toolbarElements').should('have.text', 3);
196-
cy.findByTestId('overflowElements').should('have.text', 3);
197+
cy.findByTestId('overflowElements').should('have.text', 2);
197198

198199
cy.findByText('Add').click();
199200

200201
cy.get('@overflowChangeSpy').should('have.callCount', 6);
201202
cy.findByTestId('toolbarElements').should('have.text', 3);
202-
cy.findByTestId('overflowElements').should('have.text', 4);
203+
cy.findByTestId('overflowElements').should('have.text', 3);
203204

204205
cy.findByText('Add').click();
205206
cy.findByText('Add').click();
@@ -209,13 +210,13 @@ describe('Toolbar', () => {
209210

210211
cy.get('@overflowChangeSpy').should('have.callCount', 11);
211212
cy.findByTestId('toolbarElements').should('have.text', 3);
212-
cy.findByTestId('overflowElements').should('have.text', 9);
213+
cy.findByTestId('overflowElements').should('have.text', 8);
213214

214215
cy.findByText('Remove').click();
215216

216217
cy.get('@overflowChangeSpy').should('have.callCount', 12);
217218
cy.findByTestId('toolbarElements').should('have.text', 3);
218-
cy.findByTestId('overflowElements').should('have.text', 8);
219+
cy.findByTestId('overflowElements').should('have.text', 7);
219220

220221
cy.findByText('Remove').click();
221222
cy.findByText('Remove').click();
@@ -225,14 +226,14 @@ describe('Toolbar', () => {
225226

226227
cy.get('@overflowChangeSpy').should('have.callCount', 17);
227228
cy.findByTestId('toolbarElements').should('have.text', 3);
228-
cy.findByTestId('overflowElements').should('have.text', 3);
229+
cy.findByTestId('overflowElements').should('have.text', 2);
229230

230231
cy.get(`[ui5-toggle-button]`).click();
231232

232-
// ToolbarSpacers should not be visible in the popover
233+
// ToolbarSpacers should not be rendered in the popover
233234
cy.get('[data-component-name="ToolbarOverflowPopover"]')
234235
.findByTestId('spacer2')
235-
.should('not.be.visible', { timeout: 100 });
236+
.should('not.exist', { timeout: 100 });
236237
cy.findByTestId('spacer1').should('exist');
237238

238239
// ToolbarSeparator should be displayed with horizontal line

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,11 +396,10 @@ const Toolbar = forwardRef<HTMLDivElement, ToolbarPropTypes>((props, ref) => {
396396
>
397397
<OverflowPopover
398398
overflowPopoverRef={overflowPopoverRef}
399-
lastVisibleIndex={lastVisibleIndex}
399+
lastVisibleIndex={Math.max(lastVisibleIndex, numberOfAlwaysVisibleItems - 1)}
400400
classes={classNames}
401401
portalContainer={portalContainer}
402402
overflowContentRef={overflowContentRef}
403-
numberOfAlwaysVisibleItems={numberOfAlwaysVisibleItems}
404403
overflowButton={overflowButton}
405404
setIsMounted={setIsPopoverMounted}
406405
a11yConfig={a11yConfig}

0 commit comments

Comments
 (0)