Skip to content

Commit 29bb849

Browse files
authored
Merge pull request #3 from esmcelroy/copilot/auto-generate-preview-on-settings-change
2 parents 010a69a + bff56dc commit 29bb849

7 files changed

Lines changed: 105 additions & 33 deletions

File tree

e2e/settings.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { test, expect } from '@playwright/test'
2-
import { createLandscapePng, uploadTestImage } from './test-helpers'
32

43
test.beforeEach(async ({ page }) => {
54
await page.goto('/')

src/App.tsx

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useState, useCallback } from 'react';
1+
import { useState, useCallback, useEffect, useRef } from 'react';
22
import JSZip from 'jszip';
33
import { PhotoUpload } from './components/PhotoUpload';
44
import { PaddingSettingsPanel } from './components/PaddingSettingsPanel';
@@ -67,7 +67,12 @@ export default function App() {
6767
const [isProcessed, setIsProcessed] = useState(false);
6868
const [progress, setProgress] = useState(0);
6969
const [theme, setTheme] = useDarkMode();
70-
const [settingsChangedSinceProcess, setSettingsChangedSinceProcess] = useState(false);
70+
71+
// Ref that always holds the latest handleProcess so the debounce timer
72+
// never captures a stale closure.
73+
const handleProcessRef = useRef<() => void>(() => {});
74+
const autoProcessTimerRef = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
75+
const isInitialMountRef = useRef(true);
7176

7277
const THEME_OPTIONS: { value: Theme; icon: typeof Sun; label: string }[] = [
7378
{ value: 'light', icon: Sun, label: 'Light' },
@@ -116,7 +121,6 @@ export default function App() {
116121
if (photos.length === 0) return;
117122
setIsProcessing(true);
118123
setProgress(0);
119-
setSettingsChangedSinceProcess(false);
120124

121125
// Determine target aspect ratio
122126
let target: number;
@@ -138,6 +142,26 @@ export default function App() {
138142
setIsProcessed(true);
139143
};
140144

145+
// Keep the ref pointing at the latest handleProcess so the debounce
146+
// timer always calls the version that closes over fresh state.
147+
useEffect(() => {
148+
handleProcessRef.current = handleProcess;
149+
});
150+
151+
// Auto-process when settings change (debounced), provided photos are loaded.
152+
// photos.length is intentionally omitted from deps to avoid triggering on
153+
// photo additions/removals; handleProcess already guards against empty photos.
154+
useEffect(() => {
155+
if (isInitialMountRef.current) {
156+
isInitialMountRef.current = false;
157+
return;
158+
}
159+
if (photos.length === 0) return;
160+
clearTimeout(autoProcessTimerRef.current);
161+
autoProcessTimerRef.current = setTimeout(() => handleProcessRef.current(), 400);
162+
return () => clearTimeout(autoProcessTimerRef.current);
163+
}, [settings]); // eslint-disable-line react-hooks/exhaustive-deps
164+
141165
const handleDownloadAll = async () => {
142166
const processedPhotos = photos.filter(p => p.paddedDataUrl);
143167
if (processedPhotos.length === 0) return;
@@ -281,7 +305,6 @@ export default function App() {
281305
defaultSettings={DEFAULT_SETTINGS}
282306
onChange={s => {
283307
setSettings(s);
284-
if (isProcessed) setSettingsChangedSinceProcess(true);
285308
setIsProcessed(false);
286309
}}
287310
/>
@@ -305,11 +328,6 @@ export default function App() {
305328
</>
306329
)}
307330
</button>
308-
{settingsChangedSinceProcess && (
309-
<p className="text-xs text-amber-600 dark:text-amber-400 text-center mt-2">
310-
Settings changed — re-process to apply
311-
</p>
312-
)}
313331
</div>
314332
</div>
315333
</div>

src/__tests__/App.test.tsx

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('App', () => {
3939
addListener: vi.fn(),
4040
removeListener: vi.fn(),
4141
dispatchEvent: vi.fn(),
42-
})) as any;
42+
})) as unknown as MediaQueryList;
4343
});
4444

4545
afterEach(() => {
@@ -114,7 +114,7 @@ describe('App', () => {
114114
setTimeout(() => this.onload?.(), 0);
115115
}
116116
}
117-
global.FileReader = MockFileReader as any;
117+
global.FileReader = MockFileReader as unknown as typeof FileReader;
118118

119119
return {
120120
restore: () => { global.FileReader = origFileReader; },
@@ -204,4 +204,60 @@ describe('App', () => {
204204

205205
restore();
206206
});
207+
208+
// --- Auto-process on settings change ---
209+
it('does not show "Settings changed" warning (auto-process replaces it)', async () => {
210+
const user = userEvent.setup();
211+
const mockFile = await setupMocksAndUpload();
212+
const { padImageToAspectRatio } = await import('../lib/imageUtils');
213+
(padImageToAspectRatio as ReturnType<typeof vi.fn>).mockResolvedValue('data:image/png;base64,padded');
214+
const { restore } = setupUploadMocks();
215+
216+
render(<App />);
217+
218+
// Upload and process
219+
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
220+
fireEvent.change(input, { target: { files: [mockFile] } });
221+
await waitFor(() => expect(screen.getByText(/1 photo/)).toBeInTheDocument());
222+
await user.click(screen.getByText('Process Images').closest('button')!);
223+
await waitFor(() => expect(screen.getByText('Download All as ZIP')).toBeInTheDocument());
224+
225+
// Change a setting – the old warning must never appear
226+
const resetButton = screen.getByTitle('Reset to defaults');
227+
await user.click(resetButton);
228+
expect(screen.queryByText(/re-process to apply/i)).not.toBeInTheDocument();
229+
230+
restore();
231+
});
232+
233+
it('auto-processes after a settings change when photos are loaded', async () => {
234+
const user = userEvent.setup();
235+
const mockFile = await setupMocksAndUpload();
236+
const { padImageToAspectRatio } = await import('../lib/imageUtils');
237+
(padImageToAspectRatio as ReturnType<typeof vi.fn>).mockResolvedValue('data:image/png;base64,padded');
238+
const { restore } = setupUploadMocks();
239+
240+
render(<App />);
241+
242+
// Upload and do the initial manual process
243+
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
244+
fireEvent.change(input, { target: { files: [mockFile] } });
245+
await waitFor(() => expect(screen.getByText(/1 photo/)).toBeInTheDocument());
246+
await user.click(screen.getByText('Process Images').closest('button')!);
247+
await waitFor(() => expect(screen.getByText('Download All as ZIP')).toBeInTheDocument());
248+
249+
// Simulate a settings change by switching the output format (JPEG ≠ default PNG)
250+
await user.click(screen.getByRole('button', { name: 'jpeg' }));
251+
252+
// Download All disappears immediately (isProcessed reset to false)
253+
expect(screen.queryByText('Download All as ZIP')).not.toBeInTheDocument();
254+
255+
// After the debounce fires, auto-process re-runs and Download All reappears
256+
await waitFor(
257+
() => expect(screen.getByText('Download All as ZIP')).toBeInTheDocument(),
258+
{ timeout: 3000 },
259+
);
260+
261+
restore();
262+
}, 5000);
207263
});

src/__tests__/PhotoGrid.test.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
1+
import { describe, it, expect, vi, afterEach } from 'vitest';
22
import { render, screen, fireEvent, act } from '@testing-library/react';
33
import { PhotoGrid } from '../components/PhotoGrid';
44
import type { UploadedPhoto } from '../types';
@@ -70,7 +70,7 @@ describe('PhotoGrid', () => {
7070
const clickSpy = vi.fn()
7171
const createElementSpy = vi.spyOn(document, 'createElement').mockImplementation((tag: string) => {
7272
if (tag === 'a') {
73-
return { href: '', download: '', click: clickSpy } as any
73+
return { href: '', download: '', click: clickSpy } as unknown as HTMLAnchorElement
7474
}
7575
return originalCreateElement(tag)
7676
})
@@ -116,13 +116,13 @@ describe('PhotoGrid', () => {
116116
})
117117
globalThis.ClipboardItem = class {
118118
constructor(public items: Record<string, Blob>) {}
119-
} as any
119+
} as unknown as typeof ClipboardItem
120120

121121
const pngBlob = new Blob(['px'], { type: 'image/png' })
122122
const mockFetch = vi.fn().mockResolvedValue({
123123
blob: () => Promise.resolve(pngBlob),
124124
})
125-
window.fetch = mockFetch as any
125+
window.fetch = mockFetch as unknown as typeof fetch
126126

127127
const photos = [
128128
makePhoto({ id: 'p1', paddedDataUrl: 'data:image/png;base64,padded' }),
@@ -160,7 +160,7 @@ describe('PhotoGrid', () => {
160160
set(v: string) { downloadFilename = v },
161161
get() { return downloadFilename },
162162
})
163-
return anchor as any
163+
return anchor as unknown as HTMLAnchorElement
164164
}
165165
return originalCreateElement(tag)
166166
})
@@ -187,7 +187,7 @@ describe('PhotoGrid', () => {
187187
set(v: string) { downloadFilename = v },
188188
get() { return downloadFilename },
189189
})
190-
return anchor as any
190+
return anchor as unknown as HTMLAnchorElement
191191
}
192192
return originalCreateElement(tag)
193193
})

src/__tests__/heicUtils.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ describe('convertHeicToJpeg', () => {
117117
}])
118118
const mockCanvas = makeMockCanvas()
119119
createElementSpy = vi.spyOn(document, 'createElement').mockImplementation((tag: string) => {
120-
if (tag === 'canvas') return mockCanvas as any
120+
if (tag === 'canvas') return mockCanvas as unknown as HTMLCanvasElement
121121
return originalCreateElement(tag)
122122
})
123123
})
@@ -150,7 +150,7 @@ describe('convertHeicToJpeg', () => {
150150
createElementSpy.mockRestore()
151151
const mockCanvas = makeMockCanvas(null)
152152
createElementSpy = vi.spyOn(document, 'createElement').mockImplementation((tag: string) => {
153-
if (tag === 'canvas') return mockCanvas as any
153+
if (tag === 'canvas') return mockCanvas as unknown as HTMLCanvasElement
154154
return originalCreateElement(tag)
155155
})
156156

@@ -173,7 +173,7 @@ describe('processFilesForHeic', () => {
173173
}])
174174
const mockCanvas = makeMockCanvas()
175175
createElementSpy = vi.spyOn(document, 'createElement').mockImplementation((tag: string) => {
176-
if (tag === 'canvas') return mockCanvas as any
176+
if (tag === 'canvas') return mockCanvas as unknown as HTMLCanvasElement
177177
return originalCreateElement(tag)
178178
})
179179
})

src/__tests__/imageUtils.test.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
22
import { findMaxAspectRatio, getImageDimensions, padImageToAspectRatio, drawPatternFill, drawWatermark, getWatermarkPosition } from '../lib/imageUtils'
3-
import type { UploadedPhoto, PaddingSettings, PatternSettings, WatermarkSettings, WatermarkPosition } from '../types'
3+
import type { UploadedPhoto, PaddingSettings, PatternSettings, WatermarkSettings } from '../types'
44

55
function makePhoto(width: number, height: number, overrides?: Partial<UploadedPhoto>): UploadedPhoto {
66
return {
@@ -92,7 +92,7 @@ function installCanvasMocks() {
9292

9393
HTMLCanvasElement.prototype.getContext = vi.fn(() => mockCtx) as unknown as typeof HTMLCanvasElement.prototype.getContext
9494
HTMLCanvasElement.prototype.toDataURL = vi.fn(
95-
(type?: string, _quality?: unknown) => `data:${type ?? 'image/png'};base64,MOCK`
95+
(type?: string) => `data:${type ?? 'image/png'};base64,MOCK`
9696
)
9797
}
9898

@@ -108,13 +108,12 @@ function installImageMock(naturalWidth = 800, naturalHeight = 600) {
108108
onerror: ((err: unknown) => void) | null = null
109109

110110
constructor() {
111-
const self = this
112111
// Auto-fire onload on next microtask after src is set
113112
Object.defineProperty(this, 'src', {
114-
get: () => self._src,
115-
set(value: string) {
116-
self._src = value
117-
queueMicrotask(() => self.onload?.())
113+
get: () => this._src,
114+
set: (value: string) => {
115+
this._src = value
116+
queueMicrotask(() => this.onload?.())
118117
},
119118
})
120119
}
@@ -288,15 +287,15 @@ describe('padImageToAspectRatio', () => {
288287
it('outputs webp format with quality', async () => {
289288
const photo = makePhoto(600, 600, { dataUrl: 'data:img' })
290289
const settings = defaultSettings({ outputFormat: 'webp', outputQuality: 0.9 })
291-
const result = await padImageToAspectRatio(photo, 2, settings)
290+
await padImageToAspectRatio(photo, 2, settings)
292291

293292
expect(HTMLCanvasElement.prototype.toDataURL).toHaveBeenCalledWith('image/webp', 0.9)
294293
})
295294

296295
it('outputs png format without quality parameter', async () => {
297296
const photo = makePhoto(600, 600, { dataUrl: 'data:img' })
298297
const settings = defaultSettings({ outputFormat: 'png' })
299-
const result = await padImageToAspectRatio(photo, 2, settings)
298+
await padImageToAspectRatio(photo, 2, settings)
300299

301300
expect(HTMLCanvasElement.prototype.toDataURL).toHaveBeenCalledWith('image/png', undefined)
302301
})

src/__tests__/useDarkMode.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ describe('useDarkMode', () => {
3434
mockListeners.push(...listeners);
3535
// Keep reference so listeners added later are captured
3636
const origAddEventListener = mql.addEventListener as ReturnType<typeof vi.fn>;
37-
(mql as any).addEventListener = vi.fn((_event: string, handler: (e: { matches: boolean }) => void) => {
37+
(mql as unknown as Record<string, unknown>).addEventListener = vi.fn((_event: string, handler: (e: { matches: boolean }) => void) => {
3838
mockListeners.push(handler);
3939
origAddEventListener(_event, handler);
4040
});
4141
return mql;
42-
}) as any;
42+
}) as unknown as typeof window.matchMedia;
4343
});
4444

4545
afterEach(() => {
@@ -104,7 +104,7 @@ describe('useDarkMode', () => {
104104
window.matchMedia = vi.fn(() => {
105105
const { mql } = createMatchMedia(true);
106106
return mql;
107-
}) as any;
107+
}) as unknown as typeof window.matchMedia;
108108
const { result } = renderHook(() => useDarkMode());
109109
expect(result.current[0]).toBe('system');
110110
expect(result.current[2]).toBe(true);

0 commit comments

Comments
 (0)