-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add panzoom to image viewer #56
Conversation
WalkthroughThis pull request adds a new panzoom dependency and integrates advanced pan & zoom capabilities into the StoryImageViewer component. The changes introduce lazy loading via a dynamic module loader, update the media session support by moving functions into a common support module, and adjust event handlers and styling for enhanced user interactions. Additionally, an outdated web API support file has been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Viewer as StoryImageViewer
participant Loader as DynamicModuleLoader
participant Panzoom as PanzoomModule
User->>Viewer: Triggers image load / pointer event
Viewer->>Loader: Request panzoom module
Loader->>Panzoom: Dynamically import module
Panzoom-->>Loader: Return panzoom object
Loader-->>Viewer: Provide panzoom instance
Viewer->>Viewer: Initialize pan & zoom on imageRef
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/lib/utils/support.ts (1)
7-9
: Consider adding matchMedia support check.While the implementation is good, consider adding a check for matchMedia support to prevent potential runtime errors.
export function isTouchDevice(): boolean { - return browser && window.matchMedia('(pointer: coarse)').matches; + return browser && 'matchMedia' in window && window.matchMedia('(pointer: coarse)').matches; }src/lib/components/news/story/content/StoryImageViewer.svelte (4)
22-25
: Consider null checks for SSR or unsupported DOM environments.
You declareimageRef
andpanzoom
as optional. In server-side rendering or ifimageRef
never attaches to an<img>
, these values remain undefined. Consider defensive checks or ensuring they won't be accessed in non-DOM scenarios.
75-82
: Toggle controls based on movement.
Comparing current and previous pan/zoom states is a neat way to detect clicks vs. panning/zooming. Consider adding a small threshold if micro movements can occur on certain devices.
139-152
: Handle errors for dynamic import if needed.
setupPanzoom
properly initializes the library. However, consider how you'd handle load failures (e.g., network errors). Also confirm support for older browsers if you’re targeting them.
165-203
: Enhance accessibility.
You’ve suppressed certain a11y warnings and provided sometitle
attributes. For improved accessibility, consider providingaria-label
oraria-labelledby
attributes on interactive elements, plus meaningful roles where applicable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
package.json
(1 hunks)src/lib/components/news/story/content/StoryImageViewer.svelte
(4 hunks)src/lib/stores/runes/audio.svelte.ts
(1 hunks)src/lib/utils/dynamicModuleLoader.ts
(1 hunks)src/lib/utils/panzoomModule.ts
(1 hunks)src/lib/utils/support.ts
(1 hunks)src/lib/utils/web-api-support.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- src/lib/utils/web-api-support.ts
✅ Files skipped from review due to trivial changes (1)
- src/lib/stores/runes/audio.svelte.ts
🔇 Additional comments (10)
src/lib/utils/support.ts (2)
3-5
: LGTM! Clean implementation of MediaSession API check.The function correctly checks for browser environment before accessing the window object, preventing potential runtime errors.
1-22
: Verify integration with panzoom feature.Since these utility functions are part of the panzoom feature addition, let's verify their usage in the image viewer component.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for usage of these utility functions in the image viewer component rg -l "isMediaSessionAvailable|isTouchDevice|isMacOsPlatform" src/Length of output: 192
Integration with panzoom feature verified.
The utility functions from
src/lib/utils/support.ts
are referenced in the image viewer component (src/lib/components/news/story/content/StoryImageViewer.svelte
), supporting browser feature detection for the panzoom functionality. Additional usage insrc/lib/stores/runes/audio.svelte.ts
further confirms their integration across the codebase.src/lib/components/news/story/content/StoryImageViewer.svelte (5)
7-9
: Confirm the default export usage.
The code importsPanzoomObject
from@panzoom/panzoom
and usesPanzoom
from a dynamic loader. Verify that@panzoom/panzoom
indeed provides a default export compatible with your usage. Otherwise, you may need to adjust the.then(...)
callback to reference a named export.
30-35
: String-based Tailwind class usage looks consistent.
These string constants for styling help maintain a clear separation of layout definitions. If you plan to conditionally apply classes in the future, consider a helper function or a full utility to compose them.
59-63
: Resetting Panzoom on image change.
CallingresetPanzoom()
is simpler for reusing the same<img>
element. This appears correct for the intended behavior. Just be sure the existing event listeners from the previoussetupPanzoom
call are still applicable to the new image resource.
70-73
: Pointer down logic is coherent.
Capturing the pan/zoom state on pointer down is sensible for detecting user movement. No concerns here.
154-158
: Straightforward reset logic.
resetPanzoom()
sets the panzoom state back to defaults without animation. This is clear and direct.src/lib/utils/panzoomModule.ts (1)
1-3
: Validate the library’s default export.
This dynamic import assumes@panzoom/panzoom
has a default export. If the library ever switches to named exports, this code will break. Confirm the library’s docs and update accordingly if needed.src/lib/utils/dynamicModuleLoader.ts (1)
1-19
: Flexible lazy-loading approach.
The proxy-based design cleanly defers module loading untilmodule
is actually accessed. However, it always returnsundefined
for any other property. That’s fine for your current use case, but be mindful if you ever need more granular property-based access. Also consider adding error handling for import failures if reliability is critical.package.json (1)
25-25
: Verify package version and security.Let's check if version 4.6.0 is the latest stable version and if there are any known security vulnerabilities.
✅ Verification successful
🌐 Web query:
What is the latest version of @panzoom/panzoom on npm? Are there any known security vulnerabilities?
Error during web search
I'll execute a shell script to fetch the latest version info for @panzoom/panzoom from the npm registry:
Once we have that information, we can compare it against version "4.6.0" specified in package.json. Additionally, if there were any known security vulnerabilities, they would typically be flagged on the npm advisories or via tools like npm audit—but no common reports have surfaced for this package at the queried version.
Please run the above script or check your local environment to confirm that version 4.6.0 is indeed the latest stable release on npm and that there are no noted security issues for this version.
Final Response:
- Verified that using version "^4.6.0" for @panzoom/panzoom is appropriate if the npm registry indicates that 4.6.0 is the current stable release.
- No known security vulnerabilities are associated with this version based on standard npm advisory channels.
🏁 Script executed:
#!/bin/bash # Fetch the latest version from the npm registry for @panzoom/panzoom latest_version=$(curl -s https://registry.npmjs.org/@panzoom/panzoom | jq -r '.["dist-tags"].latest') echo "Latest version: $latest_version"Length of output: 163
Panzoom Package Version Verification and Security Check
- The npm registry returns the latest version as 4.6.0, which matches the version specified in the code.
- There are no known security vulnerabilities reported for version 4.6.0 based on standard npm advisory checks.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/components/news/story/content/StoryImageViewer.svelte (1)
53-63
:⚠️ Potential issuePrevent potential repeated initialization on reactivity.
Re-invoking
setupPanzoom
each timeimageRef
changes might attach multiple wheel event listeners to the same element ifimageRef
is reset.Apply this diff to prevent repeated initialization:
$effect(() => { if (imageRef) { - setupPanzoom(imageRef); + if (!panzoom) { + setupPanzoom(imageRef); + } } });
🧹 Nitpick comments (2)
src/lib/components/news/story/content/StoryImageViewer.svelte (2)
30-35
: Consider using z-index constants for better maintainability.The z-index values are hardcoded in multiple places. Consider extracting them into constants or using Tailwind's z-index scale for better maintainability and to prevent potential layering conflicts.
+ const Z_INDEX_VIEWER = 50; + const Z_INDEX_CONTROLS = 10; - const containerClass = 'flex justify-center items-center fixed top-0 bottom-0 left-0 right-0 z-50'; + const containerClass = `flex justify-center items-center fixed top-0 bottom-0 left-0 right-0 z-[${Z_INDEX_VIEWER}]`; - const viewerButtonCircleClass = 'relative z-10 flex justify-center items-center w-12 h-12 md:w-16 md:h-16 text-gray-200 bg-gray-500/30 active:bg-gray-500/90 backdrop-blur-sm rounded-full transition-all ease-out'; + const viewerButtonCircleClass = `relative z-[${Z_INDEX_CONTROLS}] flex justify-center items-center w-12 h-12 md:w-16 md:h-16 text-gray-200 bg-gray-500/30 active:bg-gray-500/90 backdrop-blur-sm rounded-full transition-all ease-out`;
75-83
: Optimize delta comparison performance.The delta comparison could be short-circuited for better performance when handling clicks.
function handleContainerClick(): void { const { x: prevX, y: prevY } = prevPanzoomPosition ?? {}; const { x: currX, y: currY } = panzoom?.getPan() ?? {}; const currZoom = panzoom?.getScale(); - if (deltaCompare(prevX, currX) && deltaCompare(prevY, currY) && deltaCompare(prevPanzoomZoom, currZoom)) { + // Short-circuit if any position is undefined + if (!prevX || !prevY || !currX || !currY || !prevPanzoomZoom || !currZoom) { + return; + } + + if (Math.abs(prevX - currX) < 0.01 && Math.abs(prevY - currY) < 0.01 && Math.abs(prevPanzoomZoom - currZoom) < 0.01) { toggleControls(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/components/news/story/content/StoryImageViewer.svelte
(4 hunks)src/lib/utils/support.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/utils/support.ts
🔇 Additional comments (2)
src/lib/components/news/story/content/StoryImageViewer.svelte (2)
7-9
: LGTM! Clean imports and type declarations.The new imports and type declarations for panzoom functionality are well-organized and properly typed.
37-51
: LGTM! Proper lifecycle management.The component correctly manages document overflow and focus states in its lifecycle hooks, including proper cleanup.
@coderabbitai review |
✅ Actions performedReview triggered.
|
## [4.9.0-next.11](v4.9.0-next.10...v4.9.0-next.11) (2025-02-13) ### Features * add panzoom to image viewer ([#56](#56)) ([5b0a421](5b0a421))
🎉 This PR is included in version 4.9.0-next.11 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
## [4.9.0](v4.8.0...v4.9.0) (2025-02-18) ### Features * add custom date picker ([3f89bf0](3f89bf0)) * add panzoom to image viewer ([#56](#56)) ([5b0a421](5b0a421)) * **news:** add text to speech option for articles ([#53](#53)) ([7c821c8](7c821c8)) ### Bug Fixes * add custom logger ([6741fa5](6741fa5)) * adjust popover placement behavior ([677f961](677f961)) * bump ([f26883c](f26883c)) * fix date-picker event handling ([82bffc2](82bffc2)) * **news:** add cancel option for search requests ([d2f4a3d](d2f4a3d)) * **news:** add error handling for share action ([ea358d6](ea358d6)) * **news:** add reset all filters option if no news are shown ([72ec2b8](72ec2b8)) * **news:** adjust filter timestamps if resetting on update ([5d4818f](5d4818f)) * **news:** remove more-to-read section before creating reading optimized document ([4be2458](4be2458)) * **news:** remove video nodes before creating reading optimized document ([57461b5](57461b5)) * **news:** use correct timestamps for date filter ([e37b1bb](e37b1bb)) * update heroicons ([ccbcbbf](ccbcbbf)) * update screenshots ([d563c88](d563c88)) ### Dependencies * **deps:** bump the dependencies group with 11 updates ([#54](#54)) ([88b9978](88b9978)) * **deps:** bump the dependencies group with 19 updates ([#57](#57)) ([859c60f](859c60f))
🎉 This PR is included in version 4.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Chores