Feature/Support Zooming and Panning - #4828
Conversation
|
I just took a quick glance on my phone and it seems like everything is delegated to a 400 loc directive with no documentation. It would be helpful to provide some documentation for the directive. |
There was a problem hiding this comment.
Pull request overview
This PR introduces app-managed zooming and panning for manga reader renderers to avoid browser-level zoom side effects (UI scaling and unintended pagination), addressing the pinch-zoom/swipe interaction issues described in #2280 and #4658.
Changes:
- Added a new
ImageZoomDirectiveto implement wheel/pinch zoom and pan handling at the element level. - Integrated
appImageZoominto single/double/canvas renderers and the webtoon (infinite scroller) path. - Added an
image-zoom-activeoverflow lock style on reader containers to prevent scroll/navigation while zoomed.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/single-renderer/single-renderer.component.html | Applies appImageZoom to the single-image <img> element with a reset key. |
| UI/Web/src/app/manga-reader/_components/manga-reader/manga-reader.component.scss | Adds .image-zoom-active overflow locking for .reading-area / .reader. |
| UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.html | Applies appImageZoom to the webtoon scroller container. |
| UI/Web/src/app/manga-reader/_components/double-reverse-renderer/double-reverse-renderer.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/double-reverse-renderer/double-reverse-renderer.component.html | Applies appImageZoom to the double-reverse image container. |
| UI/Web/src/app/manga-reader/_components/double-renderer/double-renderer.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/double-renderer/double-renderer.component.html | Applies appImageZoom to the double image container. |
| UI/Web/src/app/manga-reader/_components/double-renderer-no-cover/double-no-cover-renderer.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/double-renderer-no-cover/double-no-cover-renderer.component.html | Applies appImageZoom to the double-no-cover image container. |
| UI/Web/src/app/manga-reader/_components/canvas-renderer/canvas-renderer.component.ts | Registers ImageZoomDirective in standalone component imports. |
| UI/Web/src/app/manga-reader/_components/canvas-renderer/canvas-renderer.component.html | Applies appImageZoom to the <canvas> element. |
| UI/Web/src/app/_directives/image-zoom.directive.ts | New directive implementing zoom/pan behavior and pagination suppression. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| (triggered)="loadPrevChapter.emit()" /> | ||
|
|
||
| <div #scroller tabindex="0" [style.outline]="'none'" infinite-scroll [infiniteScrollDistance]="1" [infiniteScrollThrottle]="50"> | ||
| <div #scroller appImageZoom [zoomResetKey]="webtoonImages" [lockScroll]="false" [recenterOnZoomOut]="false" tabindex="0" [style.outline]="'none'" infinite-scroll [infiniteScrollDistance]="1" [infiniteScrollThrottle]="50"> |
| private updateOverflowState(): void { | ||
| if (!this.lockScroll) return; | ||
|
|
||
| let ancestor = this.element.nativeElement.parentElement; | ||
| while (ancestor) { | ||
| if (ancestor.matches('.reading-area, .reader')) { | ||
| ancestor.classList.toggle('image-zoom-active', this.isZoomedIn()); | ||
| } | ||
| ancestor = ancestor.parentElement; | ||
| } | ||
| } |
|
Hey, thanks for the review! I may have gone a bit overboard with the documentation, as now every function & property has a comment, so let me know if you want me to pare it down a bit. My initial testing was pretty much all on desktop, so I had missed some edge cases on mobile - I've fixed the webtoon scrolling issue, and a couple other things I noticed as I did more thorough functional testing. I also clamped panning to the image bounds, so once you're zoomed in enough for the image to cover the screen you should no longer see any black regions. One thing I want to call out is that I have intentionally hidden the scrollbars when zoomed in. I decided to do this to prevent weird interactions between scrolling and dragging to pan around the image together. I'm not sure how important this is, so please advise - I can work on getting scrollbars to behave correctly if it's a necessity, otherwise I think just not zooming in at all and viewing the image with image scaling set to "width" is a user's best bet for desktop usage. |
|
Thanks, I'll try to test this soon. I'll let you know about the scrollbar, but I think it should be fine if the UX feels right. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
UI/Web/src/app/_directives/image-zoom.directive.ts:383
- recenterOnZoomOut is exposed as an
@Inputand is set by the infinite scroller template, but it’s not actually used: zooming all the way out always resets translateX/translateY to 0. This makes [recenterOnZoomOut] ineffective.
// User has zoomed all the way out, reset translation
if (nextScaleClamped === 1) {
this.scale = 1;
this.translateX = 0;
this.translateY = 0;
UI/Web/src/app/manga-reader/_components/infinite-scroller/infinite-scroller.component.html:24
- [zoomResetKey] is currently bound to the BehaviorSubject instance (webtoonImages), which keeps the same object reference as chapters/pages load. That means the ImageZoomDirective reset() won’t run on chapter changes, so zoom/pan state can unintentionally carry over. Bind zoomResetKey to something that actually changes per chapter (e.g., chapterId).
<div #scroller appImageZoom [zoomResetKey]="webtoonImages" [lockScroll]="false" [recenterOnZoomOut]="false" tabindex="0" [style.outline]="'none'" [style.touch-action]="'pan-y'" infinite-scroll [infiniteScrollDistance]="1" [infiniteScrollThrottle]="50">


Changed