Skip to content

Commit 03c5cdc

Browse files
JesmoDeviOvergaard
andauthored
fix(uui-tab-group): async tabs are not calculated correctly and have memory leaks (#726)
* fix button placement * rename classes * fix async story * add resize observer to all tabs * fix memory leak and performance issues * flex stories * revert display: block --------- Co-authored-by: Jacob Overgaard <[email protected]>
1 parent 9b64e2d commit 03c5cdc

File tree

2 files changed

+155
-36
lines changed

2 files changed

+155
-36
lines changed

packages/uui-tabs/lib/uui-tab-group.element.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ export class UUITabGroupElement extends LitElement {
5555
#visibilityBreakpoints: number[] = [];
5656

5757
#resizeObserver = new ResizeObserver(this.#onResize.bind(this));
58+
#tabResizeObservers: ResizeObserver[] = [];
59+
60+
#breakPointCalculationInProgress = false;
5861

5962
connectedCallback() {
6063
super.connectedCallback();
@@ -70,6 +73,7 @@ export class UUITabGroupElement extends LitElement {
7073
disconnectedCallback() {
7174
super.disconnectedCallback();
7275
this.#resizeObserver.unobserve(this);
76+
this.#cleanupTabs();
7377
}
7478

7579
#onResize(entries: ResizeObserverEntry[]) {
@@ -85,15 +89,26 @@ export class UUITabGroupElement extends LitElement {
8589
}
8690
}
8791

88-
#onSlotChange() {
92+
#cleanupTabs() {
8993
this.#tabElements.forEach(el => {
9094
el.removeEventListener('click', this.#onTabClicked);
95+
this.#tabResizeObservers.forEach(observer => observer.disconnect());
9196
});
97+
this.#tabResizeObservers.length = 0;
98+
}
99+
100+
#onSlotChange() {
101+
this.#cleanupTabs();
92102

93103
this.#setTabArray();
94104

95105
this.#tabElements.forEach(el => {
96106
el.addEventListener('click', this.#onTabClicked);
107+
const observer = new ResizeObserver(
108+
this.#calculateBreakPoints.bind(this)
109+
);
110+
observer.observe(el);
111+
this.#tabResizeObservers.push(observer);
97112
});
98113
}
99114

@@ -131,9 +146,18 @@ export class UUITabGroupElement extends LitElement {
131146
};
132147

133148
async #calculateBreakPoints() {
149+
if (this.#breakPointCalculationInProgress) return;
150+
151+
// Prevent multiple calculations from happening in the same frame
152+
this.#breakPointCalculationInProgress = true;
153+
requestAnimationFrame(() => {
154+
this.#breakPointCalculationInProgress = false;
155+
});
156+
134157
// Whenever a tab is added or removed, we need to recalculate the breakpoints
135158

136159
await this.updateComplete; // Wait for the tabs to be rendered
160+
137161
const gapCSSVar = Number.parseFloat(
138162
this.style.getPropertyValue('--uui-tab-group-gap')
139163
);
@@ -164,8 +188,7 @@ export class UUITabGroupElement extends LitElement {
164188
const moreButtonWidth = this._moreButtonElement.offsetWidth;
165189

166190
const containerWithoutButtonWidth =
167-
containerWidth -
168-
(moreButtonWidth ? moreButtonWidth + this.#currentGap : 0);
191+
containerWidth - (moreButtonWidth ? moreButtonWidth : 0);
169192

170193
// Do the update
171194
// Reset the hidden tabs
@@ -237,7 +260,9 @@ export class UUITabGroupElement extends LitElement {
237260
render() {
238261
return html`
239262
<div id="main">
240-
<slot @slotchange=${this.#onSlotChange}></slot>
263+
<div id="grid">
264+
<slot @slotchange=${this.#onSlotChange}></slot>
265+
</div>
241266
<uui-button
242267
popovertarget="popover-container"
243268
style="display: none"
@@ -261,11 +286,16 @@ export class UUITabGroupElement extends LitElement {
261286
static styles = [
262287
css`
263288
:host {
264-
display: block;
265289
width: 100%;
266290
}
267291
268292
#main {
293+
display: flex;
294+
justify-content: space-between;
295+
}
296+
297+
#grid {
298+
width: 1fr;
269299
display: flex;
270300
height: 100%;
271301
min-height: 48px;

packages/uui-tabs/lib/uui-tabs.story.ts

Lines changed: 120 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -225,40 +225,51 @@ WithGap.parameters = {
225225
},
226226
};
227227

228-
export const FlexLayout: Story = props => html`
229-
<h3>Tabs implemented into Flex-box scenario</h3>
230-
<p>
231-
In this case we want the input to grow and the tabs to take up the remaining
232-
space:
233-
</p>
234-
<uui-icon-registry-essential>
235-
<div
236-
style="display: flex; outline: 1px solid black; max-width: 800px; height: 100%; align-items: center; padding-left: 12px;">
237-
<uui-input style="flex-grow: 1; min-width: 200px"></uui-input>
238-
<uui-tab-group
239-
dropdown-direction="horizontal"
240-
style="
241-
flex-grow: 1;
228+
export const Async: Story = props => {
229+
// async insert tabs after 1 second
230+
setTimeout(() => {
231+
const tabs = document.querySelector('uui-tab-group');
232+
233+
if (!tabs) return;
234+
235+
const tab = document.createElement('uui-tab');
236+
tab.label = 'Async';
237+
tab.innerHTML = 'Async';
238+
tabs.appendChild(tab);
239+
240+
setTimeout(() => {
241+
tab.innerHTML = 'Async more text';
242+
}, 1000);
243+
}, 1000);
244+
245+
return html`
246+
<uui-icon-registry-essential>
247+
<div style="display: flex">
248+
<uui-tab-group
249+
dropdown-direction="horizontal"
250+
style="
251+
margin: auto;
242252
--uui-tab-group-gap: 25px;
243253
font-size: var(--uui-type-small-size);
244254
${props.inlineStyles}">
245-
<uui-tab label="content">
246-
<uui-icon slot="icon" name="document"></uui-icon>
247-
Content
248-
</uui-tab>
249-
<uui-tab active label="packages">
250-
<uui-icon slot="icon" name="settings"></uui-icon>
251-
Packages
252-
</uui-tab>
253-
<uui-tab label="media">
254-
<uui-icon slot="icon" name="picture"></uui-icon>
255-
Media
256-
</uui-tab>
257-
</uui-tab-group>
258-
</div>
259-
</uui-icon-registry-essential>
260-
`;
261-
FlexLayout.parameters = {
255+
<uui-tab label="content">
256+
<uui-icon slot="icon" name="document"></uui-icon>
257+
Content
258+
</uui-tab>
259+
<uui-tab active label="packages">
260+
<uui-icon slot="icon" name="settings"></uui-icon>
261+
Packages
262+
</uui-tab>
263+
<uui-tab label="media">
264+
<uui-icon slot="icon" name="picture"></uui-icon>
265+
Media
266+
</uui-tab>
267+
</uui-tab-group>
268+
</div>
269+
</uui-icon-registry-essential>
270+
`;
271+
};
272+
Async.parameters = {
262273
docs: {
263274
source: {
264275
code: `
@@ -279,3 +290,81 @@ FlexLayout.parameters = {
279290
},
280291
},
281292
};
293+
294+
export const CenterAlign: Story = props => html`
295+
<h3>Tabs implemented into Flex-box scenario</h3>
296+
<p>Here the tab group is center aligned in a flex-box container.</p>
297+
<uui-icon-registry-essential>
298+
<div style="display: flex;">
299+
<uui-tab-group
300+
dropdown-direction="horizontal"
301+
style="
302+
margin: auto;
303+
--uui-tab-group-gap: 25px;
304+
font-size: var(--uui-type-small-size);
305+
${props.inlineStyles}">
306+
<uui-tab label="content">Content</uui-tab>
307+
<uui-tab active label="packages">Packages</uui-tab>
308+
<uui-tab label="media">Media</uui-tab>
309+
<uui-tab label="settings">Settings</uui-tab>
310+
<uui-tab label="translations">Translations</uui-tab>
311+
</uui-tab-group>
312+
</div>
313+
</uui-icon-registry-essential>
314+
`;
315+
CenterAlign.parameters = {
316+
docs: {
317+
source: {
318+
code: `
319+
<div style="display: flex">
320+
<uui-tab-group style="margin: auto">
321+
<uui-tab label="content">Content</uui-tab>
322+
<uui-tab active label="packages">Packages</uui-tab>
323+
<uui-tab label="media">Media</uui-tab>
324+
<uui-tab label="settings">Settings</uui-tab>
325+
<uui-tab label="translations">Translations</uui-tab>
326+
</uui-tab-group>
327+
</div>
328+
`,
329+
},
330+
},
331+
};
332+
333+
export const RightAlign: Story = props => html`
334+
<h3>Tabs implemented into Flex-box scenario</h3>
335+
<p>Here the tab group is right aligned in a flex-box container.</p>
336+
<uui-icon-registry-essential>
337+
<div style="display: flex;">
338+
<uui-tab-group
339+
dropdown-direction="horizontal"
340+
style="
341+
margin-left: auto;
342+
--uui-tab-group-gap: 25px;
343+
font-size: var(--uui-type-small-size);
344+
${props.inlineStyles}">
345+
<uui-tab label="content">Content</uui-tab>
346+
<uui-tab active label="packages">Packages</uui-tab>
347+
<uui-tab label="media">Media</uui-tab>
348+
<uui-tab label="settings">Settings</uui-tab>
349+
<uui-tab label="translations">Translations</uui-tab>
350+
</uui-tab-group>
351+
</div>
352+
</uui-icon-registry-essential>
353+
`;
354+
RightAlign.parameters = {
355+
docs: {
356+
source: {
357+
code: `
358+
<div style="display: flex">
359+
<uui-tab-group style="margin: auto">
360+
<uui-tab label="content">Content</uui-tab>
361+
<uui-tab active label="packages">Packages</uui-tab>
362+
<uui-tab label="media">Media</uui-tab>
363+
<uui-tab label="settings">Settings</uui-tab>
364+
<uui-tab label="translations">Translations</uui-tab>
365+
</uui-tab-group>
366+
</div>
367+
`,
368+
},
369+
},
370+
};

0 commit comments

Comments
 (0)