Skip to content

Commit d36d18a

Browse files
milanofthemilanofthe
authored
Merge pull request #287 from pathsim/chore/audit-fixes
Audit fixes: memory leaks, perf, dialog/backend consolidation
2 parents b32fca8 + f43302c commit d36d18a

32 files changed

Lines changed: 795 additions & 858 deletions

‎src/lib/actions/hoverDetail.svelte.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* Reusable hover-detail state machine.
3+
*
4+
* Drives a "detail column" that appears next to a list when the user hovers
5+
* an item. Brushing past tiles on the way to the detail column shouldn't
6+
* flip the content, so we delay open/switch/close transitions.
7+
*
8+
* Used by NodeLibrary and EventsPanel — keep the timing identical so both
9+
* panels feel the same.
10+
*/
11+
12+
const DEFAULT_OPEN_DELAY = 250;
13+
const DEFAULT_SWITCH_DELAY = 200;
14+
const DEFAULT_CLOSE_DELAY = 120;
15+
16+
export interface HoverDetailOptions<T> {
17+
/** Called whenever the visible detail item changes (including to null). */
18+
onChange?: (item: T | null) => void;
19+
/** Called when the detail column should appear or disappear. */
20+
onVisibleChange?: (visible: boolean) => void;
21+
openDelay?: number;
22+
switchDelay?: number;
23+
closeDelay?: number;
24+
}
25+
26+
export function createHoverDetail<T>(opts: HoverDetailOptions<T> = {}) {
27+
const openDelay = opts.openDelay ?? DEFAULT_OPEN_DELAY;
28+
const switchDelay = opts.switchDelay ?? DEFAULT_SWITCH_DELAY;
29+
const closeDelay = opts.closeDelay ?? DEFAULT_CLOSE_DELAY;
30+
31+
let hoveredItem = $state<T | null>(null);
32+
let openTimer: ReturnType<typeof setTimeout> | null = null;
33+
let switchTimer: ReturnType<typeof setTimeout> | null = null;
34+
let closeTimer: ReturnType<typeof setTimeout> | null = null;
35+
36+
function clearOpenTimer() {
37+
if (openTimer !== null) {
38+
clearTimeout(openTimer);
39+
openTimer = null;
40+
}
41+
}
42+
function clearSwitchTimer() {
43+
if (switchTimer !== null) {
44+
clearTimeout(switchTimer);
45+
switchTimer = null;
46+
}
47+
}
48+
function clearCloseTimer() {
49+
if (closeTimer !== null) {
50+
clearTimeout(closeTimer);
51+
closeTimer = null;
52+
}
53+
}
54+
function clearAll() {
55+
clearOpenTimer();
56+
clearSwitchTimer();
57+
clearCloseTimer();
58+
}
59+
60+
function handleEnter(item: T) {
61+
clearCloseTimer();
62+
if (hoveredItem === item) {
63+
clearSwitchTimer();
64+
return;
65+
}
66+
if (hoveredItem !== null) {
67+
clearSwitchTimer();
68+
switchTimer = setTimeout(() => {
69+
switchTimer = null;
70+
hoveredItem = item;
71+
opts.onChange?.(item);
72+
}, switchDelay);
73+
return;
74+
}
75+
clearOpenTimer();
76+
openTimer = setTimeout(() => {
77+
openTimer = null;
78+
hoveredItem = item;
79+
opts.onChange?.(item);
80+
opts.onVisibleChange?.(true);
81+
}, openDelay);
82+
}
83+
84+
function handleLeave() {
85+
clearOpenTimer();
86+
clearSwitchTimer();
87+
if (hoveredItem === null) return;
88+
clearCloseTimer();
89+
closeTimer = setTimeout(() => {
90+
closeTimer = null;
91+
hoveredItem = null;
92+
opts.onChange?.(null);
93+
opts.onVisibleChange?.(false);
94+
}, closeDelay);
95+
}
96+
97+
function hideNow() {
98+
clearAll();
99+
const wasShown = hoveredItem !== null;
100+
hoveredItem = null;
101+
if (wasShown) {
102+
opts.onChange?.(null);
103+
opts.onVisibleChange?.(false);
104+
}
105+
}
106+
107+
function keepAlive() {
108+
clearCloseTimer();
109+
clearSwitchTimer();
110+
}
111+
112+
return {
113+
get hovered() {
114+
return hoveredItem;
115+
},
116+
handleEnter,
117+
handleLeave,
118+
hideNow,
119+
keepAlive,
120+
dismiss: handleLeave,
121+
cleanup: clearAll
122+
};
123+
}

‎src/lib/components/ConfirmationModal.svelte‎

Lines changed: 37 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
<script lang="ts">
2-
import { fade, scale } from 'svelte/transition';
3-
import { cubicOut } from 'svelte/easing';
2+
import { onDestroy } from 'svelte';
43
import { confirmationStore, type ConfirmationOptions } from '$lib/stores/confirmation';
54
import Icon from '$lib/components/icons/Icon.svelte';
5+
import DialogShell from '$lib/components/dialogs/shared/DialogShell.svelte';
66
77
let state = $state<{
88
open: boolean;
99
options: ConfirmationOptions | null;
1010
}>({ open: false, options: null });
1111
12-
confirmationStore.subscribe((s) => {
12+
const unsubscribe = confirmationStore.subscribe((s) => {
1313
state = { open: s.open, options: s.options };
1414
});
15+
onDestroy(unsubscribe);
1516
1617
function handleConfirm() {
1718
confirmationStore.confirm();
@@ -21,64 +22,47 @@
2122
confirmationStore.cancel();
2223
}
2324
24-
function handleBackdropClick(event: MouseEvent) {
25-
if (event.target === event.currentTarget) {
26-
handleCancel();
27-
}
28-
}
29-
30-
function handleKeydown(event: KeyboardEvent) {
31-
if (!state.open) return;
32-
if (event.key === 'Escape') {
33-
handleCancel();
34-
} else if (event.key === 'Enter') {
25+
function handleEnterKey(event: KeyboardEvent) {
26+
if (state.open && event.key === 'Enter') {
3527
handleConfirm();
3628
}
3729
}
3830
</script>
3931

40-
<svelte:window onkeydown={handleKeydown} />
41-
42-
{#if state.open && state.options}
43-
<div
44-
class="dialog-backdrop"
45-
onclick={handleBackdropClick}
46-
transition:fade={{ duration: 150 }}
47-
role="presentation"
48-
>
49-
<div
50-
class="confirmation-dialog glass-panel"
51-
transition:scale={{ start: 0.95, duration: 150, easing: cubicOut }}
52-
role="alertdialog"
53-
aria-modal="true"
54-
aria-labelledby="confirmation-title"
55-
aria-describedby="confirmation-message"
56-
>
57-
<div class="dialog-header">
58-
<span id="confirmation-title">{state.options.title}</span>
59-
<button class="icon-btn" onclick={handleCancel} aria-label="Close">
60-
<Icon name="x" size={16} />
61-
</button>
62-
</div>
63-
64-
<div class="dialog-body">
65-
<p id="confirmation-message">{state.options.message}</p>
66-
</div>
67-
68-
<div class="dialog-actions">
69-
<button class="ghost" onclick={handleCancel}>
70-
{state.options.cancelText}
71-
</button>
72-
<button onclick={handleConfirm}>
73-
{state.options.confirmText}
74-
</button>
75-
</div>
32+
<svelte:window onkeydown={handleEnterKey} />
33+
34+
<DialogShell
35+
open={state.open && state.options !== null}
36+
onClose={handleCancel}
37+
ariaLabelledby="confirmation-title"
38+
role="alertdialog"
39+
dialogClass="confirmation-dialog glass-panel"
40+
>
41+
{#if state.options}
42+
<div class="dialog-header">
43+
<span id="confirmation-title">{state.options.title}</span>
44+
<button class="icon-btn" onclick={handleCancel} aria-label="Close">
45+
<Icon name="x" size={16} />
46+
</button>
47+
</div>
48+
49+
<div class="dialog-body">
50+
<p id="confirmation-message">{state.options.message}</p>
51+
</div>
52+
53+
<div class="dialog-actions">
54+
<button class="ghost" onclick={handleCancel}>
55+
{state.options.cancelText}
56+
</button>
57+
<button onclick={handleConfirm}>
58+
{state.options.confirmText}
59+
</button>
7660
</div>
77-
</div>
78-
{/if}
61+
{/if}
62+
</DialogShell>
7963

8064
<style>
81-
.confirmation-dialog {
65+
:global(.confirmation-dialog) {
8266
width: 90%;
8367
max-width: 320px;
8468
display: flex;

‎src/lib/components/FlowCanvas.svelte‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import { nodeRegistry } from '$lib/nodes';
3737
import { NODE_TYPES } from '$lib/constants/nodeTypes';
3838
import { GRID_SIZE, SNAP_GRID, BACKGROUND_GAP } from '$lib/constants/grid';
39+
import { DEFAULT_NODE_WIDTH, DEFAULT_NODE_HEIGHT } from '$lib/constants/dimensions';
40+
import { shallowEqualArray, shallowEqualRecord } from '$lib/utils/shallowEqual';
3941
import type { NodeInstance, Connection, Annotation } from '$lib/nodes/types';
4042
import type { EventInstance } from '$lib/events/types';
4143
@@ -273,8 +275,8 @@
273275
if (portIndex >= ports.length) return null;
274276
275277
const rotation = (nodeData.params?.['_rotation'] as number) || 0;
276-
const width = node.measured?.width ?? node.width ?? 80;
277-
const height = node.measured?.height ?? node.height ?? 40;
278+
const width = node.measured?.width ?? node.width ?? DEFAULT_NODE_WIDTH;
279+
const height = node.measured?.height ?? node.height ?? DEFAULT_NODE_HEIGHT;
278280
279281
// Calculate port offset from center based on rotation
280282
const portCount = ports.length;
@@ -547,8 +549,8 @@
547549
}
548550
if (currentData.name !== gn.name) return true;
549551
if (currentData.color !== gn.color) return true;
550-
if (JSON.stringify(currentData.params) !== JSON.stringify(gn.params)) return true;
551-
if (JSON.stringify(currentData.pinnedParams) !== JSON.stringify(gn.pinnedParams)) return true;
552+
if (!shallowEqualRecord(currentData.params, gn.params)) return true;
553+
if (!shallowEqualArray(currentData.pinnedParams, gn.pinnedParams)) return true;
552554
return false;
553555
});
554556
@@ -738,8 +740,8 @@
738740
changedNodeIds.add(node.id);
739741
740742
// Incrementally update grid obstacle for this node
741-
const width = node.measured?.width ?? node.width ?? 80;
742-
const height = node.measured?.height ?? node.height ?? 40;
743+
const width = node.measured?.width ?? node.width ?? DEFAULT_NODE_WIDTH;
744+
const height = node.measured?.height ?? node.height ?? DEFAULT_NODE_HEIGHT;
743745
routingStore.updateNodeBounds(node.id, {
744746
x: snappedX - width / 2,
745747
y: snappedY - height / 2,

‎src/lib/components/WelcomeModal.svelte‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@
179179
<img
180180
src="{base}/examples/screenshots/{example.basename}-{isDark ? 'dark' : 'light'}.png"
181181
alt="{example.name} preview"
182+
loading="lazy"
183+
decoding="async"
182184
onerror={(e) => { (e.currentTarget as HTMLImageElement).style.display = 'none'; }}
183185
/>
184186
</div>

‎src/lib/components/dialogs/BlockPropertiesDialog.svelte‎

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
<script lang="ts">
22
import { onDestroy } from 'svelte';
3-
import { fade, scale } from 'svelte/transition';
4-
import { cubicOut } from 'svelte/easing';
53
import { graphStore } from '$lib/stores/graph';
64
import { historyStore } from '$lib/stores/history';
75
import { nodeDialogStore, closeNodeDialog } from '$lib/stores/nodeDialog';
@@ -15,6 +13,7 @@
1513
import { paramInput } from '$lib/actions/paramInput';
1614
import { loadCodeMirrorModules, createEditorExtensions, type CodeMirrorModules } from '$lib/utils/codemirror';
1715
import ColorPicker from './shared/ColorPicker.svelte';
16+
import DialogShell from './shared/DialogShell.svelte';
1817
import DocumentationSection from './shared/DocumentationSection.svelte';
1918
import Icon from '$lib/components/icons/Icon.svelte';
2019
import { DEFAULT_NODE_COLOR } from '$lib/utils/colors';
@@ -76,6 +75,7 @@
7675
unsubscribeDialog();
7776
unsubscribeNodes();
7877
unsubscribeTheme();
78+
recordingData.destroy();
7979
destroyEditor();
8080
});
8181
@@ -230,27 +230,18 @@
230230
return String(value);
231231
}
232232
233-
// Handle backdrop click
234-
function handleBackdropClick(event: MouseEvent) {
235-
if (event.target === event.currentTarget) {
236-
closeNodeDialog();
237-
}
238-
}
239-
240-
// Handle escape key
241-
function handleKeydown(event: KeyboardEvent) {
242-
if (event.key === 'Escape') {
243-
closeNodeDialog();
244-
}
245-
}
246233
</script>
247234

248-
<svelte:window onkeydown={handleKeydown} />
249-
250-
{#if nodeId && node && typeDef}
251-
<div class="dialog-backdrop" onclick={handleBackdropClick} transition:fade={{ duration: 150 }} role="presentation">
252-
<div class="properties-dialog glass-panel" data-tour="dialog-properties" style="--node-color: {currentColor}" transition:scale={{ start: 0.95, duration: 150, easing: cubicOut }} role="dialog" tabindex="-1" aria-labelledby="dialog-title">
253-
<div class="dialog-header">
235+
<DialogShell
236+
open={!!(nodeId && node && typeDef)}
237+
onClose={closeNodeDialog}
238+
ariaLabelledby="dialog-title"
239+
dataTour="dialog-properties"
240+
dialogClass="properties-dialog glass-panel"
241+
dialogStyle="--node-color: {currentColor};"
242+
>
243+
{#if nodeId && node && typeDef}
244+
<div class="dialog-header">
254245
{#if showCode}
255246
<span id="dialog-title">Python Code</span>
256247
{:else}
@@ -399,14 +390,13 @@
399390
{/if}
400391
</div>
401392

402-
{#if !showCode}
403-
<div class="dialog-footer">
404-
<span class="hint">R rotate · X flip horizontal · Y flip vertical</span>
405-
</div>
406-
{/if}
407-
</div>
408-
</div>
409-
{/if}
393+
{#if !showCode}
394+
<div class="dialog-footer">
395+
<span class="hint">R rotate · X flip horizontal · Y flip vertical</span>
396+
</div>
397+
{/if}
398+
{/if}
399+
</DialogShell>
410400

411401
<style>
412402
/* Uses global .properties-dialog styles from app.css */

0 commit comments

Comments
 (0)