Viewer update and autozoom #4800
Conversation
reecebrowne
requested review from
ConnorYoh,
EthanHealy01,
Frooodle and
jbrunton96
as code owners
dosubot
bot
added
the
size:XL
stirlingbot
bot
added
v2
There was a problem hiding this comment.
Pull Request Overview
This PR implements intelligent auto-zoom functionality for the PDF viewer that automatically adjusts zoom levels based on viewport dimensions and page aspect ratios. The implementation includes a new zoom calculation utility, integration with the EmbedPDF plugin system, and support for spread mode (dual-page) viewing.
Key changes:
- Added
viewerZoom.tsutility with auto-zoom calculation logic and viewport management - Enhanced file metadata to include page dimensions for zoom calculations
- Upgraded EmbedPDF packages from
1.3.14to1.4.1across all plugins - Refactored viewer context to support spread mode notifications and improved callback patterns
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/core/utils/viewerZoom.ts |
New utility module with auto-zoom logic, page measurement, and resize handling |
frontend/src/core/utils/pageMetadata.ts |
New utility for extracting page dimensions and aspect ratios from file metadata |
frontend/src/core/utils/thumbnailUtils.ts |
Extended to capture and store page dimensions during thumbnail generation |
frontend/src/core/types/fileContext.ts |
Added width/height fields to ProcessedFilePage interface |
frontend/src/core/contexts/file/fileActions.ts |
Updated to accept and store page dimensions in metadata |
frontend/src/core/contexts/ViewerContext.tsx |
Refactored callback system using useImmediateNotifier hook, added spread update notifications |
frontend/src/core/components/viewer/ZoomAPIBridge.tsx |
Complete rewrite implementing auto-zoom with viewport tracking and spread mode awareness |
frontend/src/core/components/viewer/SpreadAPIBridge.tsx |
Added spread update notifications to trigger immediate UI updates |
frontend/src/core/components/viewer/PdfViewerToolbar.tsx |
Connected dual-page toggle to spread actions and immediate update system |
frontend/src/core/components/viewer/LocalEmbedPDF.tsx |
Changed default zoom to FitWidth mode, added data attributes for page measurement |
frontend/src/core/components/viewer/EmbedPdfViewer.tsx |
Removed unused state getters and simplified props |
frontend/package.json |
Upgraded all EmbedPDF packages to 1.4.1 |
frontend/package-lock.json |
Updated package lock with new EmbedPDF versions and peer dependency changes |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
|
|
||
| if (typeof timeoutId === 'number') { |
There was a problem hiding this comment.
The type check typeof timeoutId === 'number' is redundant since timeoutId is explicitly typed as number | undefined. In TypeScript, you can directly check if (timeoutId !== undefined) or if (timeoutId) for cleaner code.
|
|
||
| window.addEventListener('resize', handleResize); | ||
| return () => { | ||
| if (typeof timeoutId === 'number') { |
There was a problem hiding this comment.
Same redundant type check as line 170. The variable timeoutId is already typed as number | undefined, so this check can be simplified to if (timeoutId !== undefined) or just if (timeoutId).
| ]); | ||
|
|
||
| useEffect(() => { | ||
| if (!zoom || typeof zoom.onZoomChange !== 'function') { |
There was a problem hiding this comment.
The runtime check typeof zoom.onZoomChange !== 'function' suggests uncertainty about the API shape. If zoom is properly typed and onZoomChange is part of the interface, this check is unnecessary. Consider relying on TypeScript types or documenting why this runtime check is needed.
| if (!zoom || typeof zoom.onZoomChange !== 'function') { | |
| if (!zoom) { |
| if (typeof unsubscribe === 'function') { | ||
| unsubscribe(); | ||
| } |
There was a problem hiding this comment.
If onZoomChange is expected to return an unsubscribe function according to the API contract, this runtime type check is redundant. TypeScript should guarantee the type, and defensive checks can mask type system issues.
| if (typeof unsubscribe === 'function') { | |
| unsubscribe(); | |
| } | |
| unsubscribe(); |
dosubot
bot
added
size:XXL
✅ Frontend License Check PassedAll frontend licenses have been validated and no compatibility warnings were detected. The frontend license report has been updated successfully. |
🚀 V2 Auto-Deployment Complete!Your V2 PR with the new frontend/backend split architecture has been deployed! 🔗 Direct Test URL (non-SSL) http://185.252.234.121:4800 🔐 Secure HTTPS URL: https://4800.ssl.stirlingpdf.cloud This deployment will be automatically cleaned up when the PR is closed. 🔄 Auto-deployed because PR title or branch name contains V2/version2/React keywords. |
Updated embed PDF
Added Autozoom
Added file page size to metadata for use in calculations for autozoom, will come in handy elsewhere.