Skip to content

Commit f49c0bd

Browse files
committed
fix(core): memory leaks from connected callback states
- Added tests to ensure that the checked state of checkboxes and radio buttons resets to their native defaults upon calling the reset method. - Improved the handling of observers in form controls to prevent duplication after reconnecting. - Enhanced the Select component to stop observing removed options and to reset selected options to their native defaults. - Added accessibility tests for the PagePanel component to ensure compliance with axe standards. Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent 7f92252 commit f49c0bd

37 files changed

Lines changed: 741 additions & 209 deletions

projects/core/src/checkbox/checkbox.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ describe(`${Checkbox.metadata.tag} - control base behavior`, () => {
8989
expect((e as Event).composed).toBe(true);
9090
});
9191

92+
it('should reset checked state to the native default', async () => {
93+
const input = fixture.querySelector('input');
94+
input.defaultChecked = true;
95+
input.checked = false;
96+
97+
element.reset();
98+
await elementIsStable(element);
99+
100+
expect(input.checked).toBe(true);
101+
});
102+
92103
it('should disconnect observers when removed from DOM', async () => {
93104
const input = fixture.querySelector('input');
94105
expect(element.matches(':state(checked)')).toBe(false);

projects/core/src/combobox/combobox.test.lighthouse.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('combobox lighthouse report', () => {
2525
expect(report.scores.performance).toBe(100);
2626
expect(report.scores.accessibility).toBe(100);
2727
expect(report.scores.bestPractices).toBe(100);
28-
expect(report.payload.javascript.kb).toBeLessThan(36);
28+
expect(report.payload.javascript.kb).toBeLessThan(36.1);
2929
});
3030

3131
test('combobox multi select with large dataset should meet lighthouse benchmarks', async () => {
@@ -42,6 +42,6 @@ describe('combobox lighthouse report', () => {
4242

4343
expect(report.scores.performance).toBeGreaterThanOrEqual(96);
4444
expect(report.scores.bestPractices).toBe(100);
45-
expect(report.payload.javascript.kb).toBeLessThan(36);
45+
expect(report.payload.javascript.kb).toBeLessThan(36.1);
4646
});
4747
});

projects/core/src/dialog/dialog.test.lighthouse.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ describe('dialog lighthouse report', () => {
2525
expect(report.scores.performance).toBeGreaterThan(97); // bfcache
2626
expect(report.scores.accessibility).toBe(100);
2727
expect(report.scores.bestPractices).toBe(100);
28-
expect(report.payload.javascript.kb).toBeLessThan(24.8);
28+
expect(report.payload.javascript.kb).toBeLessThan(24.9);
2929
});
3030
});

projects/core/src/drawer/drawer.test.lighthouse.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,6 @@ describe('drawer lighthouse report', () => {
1818
expect(report.scores.performance).toBeGreaterThan(98); // bfcache
1919
expect(report.scores.accessibility).toBe(100);
2020
expect(report.scores.bestPractices).toBe(100);
21-
expect(report.payload.javascript.kb).toBeLessThan(25.3);
21+
expect(report.payload.javascript.kb).toBeLessThan(25.4);
2222
});
2323
});

projects/core/src/dropdown-group/dropdown-group.test.lighthouse.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,6 @@ describe('dropdown-group lighthouse report', () => {
2929
expect(report.scores.performance).toBe(100);
3030
expect(report.scores.accessibility).toBe(100);
3131
expect(report.scores.bestPractices).toBe(100);
32-
expect(report.payload.javascript.kb).toBeLessThan(24.7);
32+
expect(report.payload.javascript.kb).toBeLessThan(25);
3333
});
3434
});

projects/core/src/format-relative-time/format-relative-time.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,5 +408,19 @@ describe(FormatRelativeTime.metadata.tag, () => {
408408
const time = element.shadowRoot!.querySelector('time');
409409
expect(time!.textContent!.trim()).toBe('2023-07-27T12:00:00.000Z');
410410
});
411+
412+
it('should fall back to raw string for relative values outside Intl range', async () => {
413+
vi.useRealTimers();
414+
const now = vi.spyOn(Date, 'now').mockReturnValue(Number.NEGATIVE_INFINITY);
415+
416+
element.date = '2023-07-27T12:00:00.000Z';
417+
element.requestUpdate();
418+
await elementIsStable(element);
419+
420+
const time = element.shadowRoot!.querySelector('time');
421+
expect(time!.textContent!.trim()).toBe('2023-07-27T12:00:00.000Z');
422+
423+
now.mockRestore();
424+
});
411425
});
412426
});

projects/core/src/format-relative-time/format-relative-time.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export class FormatRelativeTime extends LitElement {
9595
const value = Math.round(absDiff / UNIT_DIVISORS[unit]);
9696
if (value < max) return { value: sign * value, unit };
9797
}
98-
throw new Error('format-relative-time: no relative time threshold matched');
98+
return { value: sign * Math.round(absDiff / UNIT_DIVISORS.year), unit: 'year' };
9999
}
100100

101101
#computeExplicitUnit(diffMs: number, unit: TimeUnitOption): number {

projects/core/src/forms/control-group/control-group.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
--color: var(--nve-sys-interaction-field-color);
66
--opacity: 1;
77
--label-color: var(--nve-sys-interaction-field-color);
8-
--label-text-transform: capitalize;
8+
--label-text-transform: none;
99
--label-font-weight: var(--nve-ref-font-weight-medium);
1010
--label-font-size: var(--nve-ref-font-size-100);
1111
--_label-width: var(--label-width, 180px);

projects/core/src/forms/control-group/control-group.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import {
1212
associateControlGroup
1313
} from '@nvidia-elements/core/internal';
1414
import type { ControlMessage } from '../control-message/control-message.js';
15-
import { setupControlStatusStates, setupControlGroupStates, inputQuery } from '../utils/states.js';
15+
import {
16+
setupControlStatusStates,
17+
setupControlGroupStates,
18+
inputQuery,
19+
type ControlStateCleanup
20+
} from '../utils/states.js';
1621
import { setupControlLayoutStates } from '../utils/layout.js';
1722
import styles from './control-group.css?inline';
1823

@@ -53,7 +58,7 @@ export class ControlGroup extends LitElement {
5358
return this.querySelectorAll ? Array.from(this.querySelectorAll<ControlMessage>('nve-control-message')) : [];
5459
}
5560

56-
#observers: (MutationObserver | ResizeObserver)[] = [];
61+
#observers: (MutationObserver | ResizeObserver | ControlStateCleanup)[] = [];
5762

5863
static styles = useStyles([styles]);
5964

projects/core/src/forms/control/control.css

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
--accent-color: var(--nve-sys-accent-secondary-background);
77
--color: var(--nve-sys-interaction-field-color);
88
--label-color: var(--nve-sys-interaction-field-color);
9-
--label-text-transform: capitalize;
9+
--label-text-transform: none;
1010
--label-font-weight: var(--nve-ref-font-weight-medium);
1111
--label-font-size: var(--nve-ref-font-size-100);
1212
--control-width: 100%;
@@ -18,10 +18,6 @@
1818
contain: content;
1919
}
2020

21-
:host([nve-control='inline']) {
22-
--control-height: auto;
23-
}
24-
2521
::slotted(*) {
2622
color-scheme: var(--nve-sys-color-scheme);
2723
font-family: inherit;
@@ -38,10 +34,6 @@
3834
align-self: center !important;
3935
}
4036

41-
::slotted(label::first-letter) {
42-
text-transform: capitalize;
43-
}
44-
4537
:host(:state(disabled)),
4638
:host(:state(disabled)) ::slotted(nve-control-message:not([status])) {
4739
--cursor: not-allowed;
@@ -194,6 +186,9 @@ slot[name='messages'] {
194186

195187
/** inline controls */
196188
:host([nve-control='inline']) {
189+
--control-height: auto;
190+
width: fit-content;
191+
197192
[internal-host] {
198193
grid-template-areas: 'input input input';
199194

0 commit comments

Comments
 (0)