Skip to content

Commit 37a33a8

Browse files
committed
🐛 fix(ui): harden dashboard layout against unknown widgets and unnecessary re-renders
- Filter unknown widget IDs in createDefaultLayoutForBreakpoint instead of crashing - Memoize responsive layouts so unchanged breakpoints preserve referential identity
1 parent 3340b7c commit 37a33a8

4 files changed

Lines changed: 133 additions & 15 deletions

File tree

ui/src/views/dashboard/dashboardWidgetLayout.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ const DEFAULT_LAYOUT: WidgetLayoutItem[] = [
8383

8484
const DEFAULT_LAYOUT_BY_ID = new Map(DEFAULT_LAYOUT.map((item) => [item.i, item] as const));
8585

86+
function getDefaultLayoutItem(widgetId: string): WidgetLayoutItem | null {
87+
const item = DEFAULT_LAYOUT_BY_ID.get(widgetId as DashboardWidgetId);
88+
return item ? { ...item } : null;
89+
}
90+
8691
function clamp(value: number, min: number, max: number): number {
8792
return Math.max(min, Math.min(max, value));
8893
}
@@ -160,19 +165,26 @@ export function createDefaultLayoutForBreakpoint(
160165
): WidgetLayoutItem[] {
161166
if (!isSingleColumnBreakpoint(breakpoint)) {
162167
return applyConstraints(
163-
order.map((id) => ({ ...DEFAULT_LAYOUT_BY_ID.get(id)! })),
168+
order
169+
.map((id) => getDefaultLayoutItem(id))
170+
.filter((item): item is WidgetLayoutItem => item !== null),
164171
breakpoint,
165172
);
166173
}
167174

168175
let nextY = 0;
169176
return applyConstraints(
170-
order.map((id) => {
171-
const fallback = DEFAULT_LAYOUT_BY_ID.get(id)!;
172-
const item: WidgetLayoutItem = { i: id, x: 0, y: nextY, w: 1, h: fallback.h };
173-
nextY += item.h;
174-
return item;
175-
}),
177+
order
178+
.map((id) => {
179+
const fallback = getDefaultLayoutItem(id);
180+
if (!fallback) {
181+
return null;
182+
}
183+
const item: WidgetLayoutItem = { i: id, x: 0, y: nextY, w: 1, h: fallback.h };
184+
nextY += item.h;
185+
return item;
186+
})
187+
.filter((item): item is WidgetLayoutItem => item !== null),
176188
breakpoint,
177189
);
178190
}

ui/src/views/dashboard/useDashboardWidgetOrder.ts

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,67 @@ function cloneLayout(layout: readonly WidgetLayoutItem[]): WidgetLayoutItem[] {
8282
return layout.map(cloneLayoutItem);
8383
}
8484

85-
function cloneResponsiveLayouts(layouts: ResponsiveWidgetLayouts): ResponsiveWidgetLayouts {
86-
const result: ResponsiveWidgetLayouts = {};
87-
for (const breakpoint of RESPONSIVE_BREAKPOINTS) {
88-
if (layouts[breakpoint]?.length) {
89-
result[breakpoint] = cloneLayout(layouts[breakpoint]!);
90-
}
85+
function layoutItemsEqual(left: WidgetLayoutItem, right: WidgetLayoutItem): boolean {
86+
return (
87+
left.i === right.i &&
88+
left.x === right.x &&
89+
left.y === right.y &&
90+
left.w === right.w &&
91+
left.h === right.h &&
92+
left.minW === right.minW &&
93+
left.minH === right.minH &&
94+
left.maxW === right.maxW &&
95+
left.maxH === right.maxH
96+
);
97+
}
98+
99+
function layoutsShallowEqual(
100+
left: readonly WidgetLayoutItem[] | undefined,
101+
right: readonly WidgetLayoutItem[] | undefined,
102+
): boolean {
103+
if (left === right) {
104+
return true;
91105
}
92-
return result;
106+
if (!left || !right || left.length !== right.length) {
107+
return false;
108+
}
109+
return left.every((item, index) => layoutItemsEqual(item, right[index]!));
110+
}
111+
112+
function createResponsiveLayoutsMemo() {
113+
let previousResult: ResponsiveWidgetLayouts = {};
114+
115+
return (layouts: ResponsiveWidgetLayouts): ResponsiveWidgetLayouts => {
116+
let changed = false;
117+
const nextResult: ResponsiveWidgetLayouts = {};
118+
119+
for (const breakpoint of RESPONSIVE_BREAKPOINTS) {
120+
const source = layouts[breakpoint];
121+
const previous = previousResult[breakpoint];
122+
123+
if (!source?.length) {
124+
if (previous?.length) {
125+
changed = true;
126+
}
127+
continue;
128+
}
129+
130+
if (layoutsShallowEqual(source, previous)) {
131+
nextResult[breakpoint] = previous;
132+
continue;
133+
}
134+
135+
nextResult[breakpoint] = cloneLayout(source);
136+
changed = true;
137+
}
138+
139+
if (!changed) {
140+
return previousResult;
141+
}
142+
143+
previousResult = nextResult;
144+
return previousResult;
145+
};
93146
}
94147

95148
function stripLayout(layout: readonly WidgetLayoutItem[]): PersistedLayoutItem[] {
@@ -248,6 +301,7 @@ export function moveWidget(
248301
}
249302

250303
export function useDashboardWidgetOrder() {
304+
const getResponsiveLayouts = createResponsiveLayoutsMemo();
251305
const widgetOrder = ref<DashboardWidgetId[]>(
252306
sanitizeWidgetOrder(preferences.dashboard.widgetOrder),
253307
);
@@ -259,7 +313,7 @@ export function useDashboardWidgetOrder() {
259313
createLayoutFromOrder(widgetOrder.value, currentBreakpoint.value),
260314
),
261315
);
262-
const responsiveLayouts = computed(() => cloneResponsiveLayouts(layoutsByBreakpoint.value));
316+
const responsiveLayouts = computed(() => getResponsiveLayouts(layoutsByBreakpoint.value));
263317
const gridInstanceKey = ref(0);
264318
const hiddenWidgets = ref<DashboardWidgetId[]>(
265319
sanitizeHiddenWidgets(preferences.dashboard.hiddenWidgets),

ui/tests/views/dashboard/dashboardWidgetLayout.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,5 +159,27 @@ describe('dashboardWidgetLayout', () => {
159159
expect(layout.every((item) => item.x === 0)).toBe(true);
160160
expect(layout.every((item) => item.w === 1)).toBe(true);
161161
});
162+
163+
test('ignores unknown widget ids on multi-column breakpoints', () => {
164+
const layout = createDefaultLayoutForBreakpoint(
165+
[...DASHBOARD_WIDGET_IDS, 'unknown-widget' as any],
166+
'lg',
167+
);
168+
169+
expect(layout).toHaveLength(DASHBOARD_WIDGET_IDS.length);
170+
expect(layout.map((item) => item.i)).toEqual(DASHBOARD_WIDGET_IDS);
171+
});
172+
173+
test('ignores unknown widget ids on single-column breakpoints', () => {
174+
const layout = createDefaultLayoutForBreakpoint(
175+
['stat-containers', 'unknown-widget' as any, 'recent-updates'],
176+
'sm',
177+
);
178+
179+
expect(layout.map((item) => item.i)).toEqual(['stat-containers', 'recent-updates']);
180+
expect(layout.map((item) => item.y)).toEqual([0, 3]);
181+
expect(layout.every((item) => item.x === 0)).toBe(true);
182+
expect(layout.every((item) => item.w === 1)).toBe(true);
183+
});
162184
});
163185
});

ui/tests/views/dashboard/useDashboardWidgetOrder.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,36 @@ describe('useDashboardWidgetOrder', () => {
357357
vi.useRealTimers();
358358
});
359359

360+
it('reuses unchanged breakpoint layouts in responsiveLayouts when persisting current breakpoint changes', async () => {
361+
vi.useFakeTimers();
362+
const mobileLayout = DASHBOARD_WIDGET_IDS.map((id, index) => ({
363+
i: id,
364+
x: 0,
365+
y: index,
366+
w: 1,
367+
h: 1,
368+
}));
369+
preferences.dashboard.gridLayouts.sm = mobileLayout;
370+
371+
const { state } = await mountWidgetOrderComposable();
372+
const beforeSm = state.responsiveLayouts.value.sm;
373+
const beforeLg = state.responsiveLayouts.value.lg;
374+
375+
const updated = state.layout.value.map((item) => ({ ...item }));
376+
updated[0] = { ...updated[0], x: 99 };
377+
state.layout.value = updated;
378+
await nextTick();
379+
380+
vi.advanceTimersByTime(300);
381+
await nextTick();
382+
383+
expect(state.responsiveLayouts.value.sm).toBe(beforeSm);
384+
expect(state.responsiveLayouts.value.lg).not.toBe(beforeLg);
385+
expect(state.responsiveLayouts.value.lg?.[0].x).toBe(99);
386+
387+
vi.useRealTimers();
388+
});
389+
360390
it('flushes pending layout persistence when the composable is disposed before debounce fires', async () => {
361391
vi.useFakeTimers();
362392
const { state, wrapper } = await mountWidgetOrderComposable();

0 commit comments

Comments
 (0)