From 4809db5f2b0da0ddadcbebb1fdae4414e9729338 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 21 May 2026 15:00:14 -0300 Subject: [PATCH 1/3] refactor: remove zoom utility module applyScaleToDocs was the only export of src/utils/zoom.ts. The responsibility is now fully absorbed by syncScaleState inside PDFElements, which is the single place that owns the canonical scale value. Removing the separate module eliminates the indirection and prevents callers from updating pagesScale without going through the centralised state-sync path. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- src/utils/zoom.ts | 10 ---------- tests/utils/zoom.spec.ts | 25 ------------------------- 2 files changed, 35 deletions(-) delete mode 100644 src/utils/zoom.ts delete mode 100644 tests/utils/zoom.spec.ts diff --git a/src/utils/zoom.ts b/src/utils/zoom.ts deleted file mode 100644 index 0e1a848..0000000 --- a/src/utils/zoom.ts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-FileCopyrightText: 2026 LibreCode coop and contributors -// SPDX-License-Identifier: AGPL-3.0-or-later - -import type { PDFDocumentEntry } from '../types' - -export function applyScaleToDocs(docs: PDFDocumentEntry[], scale: number) { - docs.forEach((doc) => { - doc.pagesScale = doc.pagesScale.map(() => scale) - }) -} diff --git a/tests/utils/zoom.spec.ts b/tests/utils/zoom.spec.ts deleted file mode 100644 index 1fa2f04..0000000 --- a/tests/utils/zoom.spec.ts +++ /dev/null @@ -1,25 +0,0 @@ -// SPDX-FileCopyrightText: 2026 LibreCode coop and contributors -// SPDX-License-Identifier: AGPL-3.0-or-later - -import { describe, it, expect } from 'vitest' -import { applyScaleToDocs } from '../../src/utils/zoom' - -const makeDoc = () => ({ - name: 'doc', - file: null, - pdfDoc: null, - numPages: 2, - pages: [Promise.resolve(), Promise.resolve()], - pageWidths: [100, 100], - pagesScale: [1, 2], - allObjects: [[], []], -}) - -describe('zoom', () => { - it('applies scale to all document pages', () => { - const docs = [makeDoc(), makeDoc()] - applyScaleToDocs(docs, 1.5) - expect(docs[0].pagesScale).toEqual([1.5, 1.5]) - expect(docs[1].pagesScale).toEqual([1.5, 1.5]) - }) -}) From e1de6779c6f55cd69bb80c77d3617c41511a66bf Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 21 May 2026 15:00:36 -0300 Subject: [PATCH 2/3] fix: centralize zoom state and guard auto-fit during pinch Before this change the component maintained two separate scale values (visualScale for intermediate wheel/pinch steps and scale as the committed value) that could diverge and cause elements to be positioned or measured with the wrong scale factor. The fix introduces: - pendingZoomScale: accumulates fast scroll-wheel ticks between RAF frames so scale always converges correctly even under rapid input. - syncScaleState(): single method that normalises, clamps, updates pagesScale on every document, and invalidates the measurement cache. All zoom paths (wheel, pinch, adjustZoomToFit, external scale prop mutation) now funnel through it. - isSyncingScale flag: prevents the scale watcher from re-entering syncScaleState when the method itself writes back to this.scale. - isTouchDevice flag: detected once at mount; used to guard unrelated touch interaction paths. on resize, preventing a concurrent auto-fit from fighting an in-progress pinch gesture. - adjustZoomToFit accepts a force=false parameter so callers that have autoFitZoom disabled can still trigger a one-shot fit. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- src/components/PDFElements.vue | 83 ++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/src/components/PDFElements.vue b/src/components/PDFElements.vue index bd7e73c..187b2fa 100644 --- a/src/components/PDFElements.vue +++ b/src/components/PDFElements.vue @@ -146,7 +146,6 @@ import DraggableElement from './DraggableElement.vue' import { readAsPDF, readAsArrayBuffer } from '../utils/asyncReader' import { clampPosition, getVisibleArea } from '../utils/geometry' import { getViewportWindow, isPageInViewport } from '../utils/pageBounds' -import { applyScaleToDocs } from '../utils/zoom' import { objectIdExistsInDoc, findObjectPageIndex, updateObjectInDoc, removeObjectFromDoc } from '../utils/objectStore' import { getCachedMeasurement } from '../utils/measurements' import type { PDFDocumentEntry, PDFElementObject, PDFElementsAddingEndedPayload } from '../types' @@ -272,9 +271,10 @@ export default defineComponent({ boundHandleTouchStart: null as ((event: TouchEvent) => void) | null, boundHandleTouchMove: null as ((event: TouchEvent) => void) | null, boundHandleTouchEnd: null as ((event: TouchEvent) => void) | null, - visualScale: this.initialScale, + pendingZoomScale: this.initialScale, + isSyncingScale: false, + isTouchDevice: false, autoFitApplied: false, - lastContainerWidth: 0, _pagesBoundingRects: markRaw({}) as Record, _pagesBoundingRectsList: markRaw([]) as { docIndex: number; pageIndex: number; rect: DOMRect }[], _pageMeasurementCache: markRaw({}) as Record, @@ -282,7 +282,19 @@ export default defineComponent({ _lastPageBoundsClientHeight: 0, } }, + watch: { + scale(newScale) { + if (this.isSyncingScale) return + this.syncScaleState(newScale) + }, + }, mounted() { + this.isTouchDevice = typeof window !== 'undefined' + && ( + (window.matchMedia?.('(pointer: coarse)').matches ?? false) + || 'ontouchstart' in window + || (typeof navigator !== 'undefined' && navigator.maxTouchPoints > 0) + ) this.boundHandleWheel = this.handleWheel.bind(this) this.boundHandleTouchStart = this.handleTouchStart.bind(this) this.boundHandleTouchMove = this.handleTouchMove.bind(this) @@ -336,6 +348,21 @@ export default defineComponent({ } }, methods: { + syncScaleState(nextScale) { + const normalizedScale = this.clampZoomScale(Number(nextScale) || 1) + if (this.scale !== normalizedScale) { + this.isSyncingScale = true + this.scale = normalizedScale + this.isSyncingScale = false + } + this.pendingZoomScale = normalizedScale + this.pdfDocuments.forEach((doc) => { + doc.pagesScale = doc.pagesScale.map(() => normalizedScale) + }) + this._pageMeasurementCache = markRaw({}) + this.cachePageBounds() + }, + async init() { if (!this.initFiles || this.initFiles.length === 0) return const docs: PDFDocumentEntry[] = [] @@ -604,7 +631,7 @@ export default defineComponent({ this.isPinching = true this.pinchStartDistance = distance - this.pinchStartScale = this.visualScale || currentScale + this.pinchStartScale = currentScale this.pinchCenter = { x: localCenterX, y: localCenterY } this.pinchAnchor = { x: (container.scrollLeft + localCenterX) / currentScale, @@ -616,8 +643,7 @@ export default defineComponent({ const container = this.$el if (!container) return - this.visualScale = nextScale - this.commitZoom() + this.commitZoom(nextScale) container.scrollLeft = Math.max(0, (this.pinchAnchor.x * nextScale) - center.x) container.scrollTop = Math.max(0, (this.pinchAnchor.y * nextScale) - center.y) this.cachePageBounds() @@ -743,9 +769,7 @@ export default defineComponent({ return fallbackRectWidth / baseWidth } } - const base = doc.pagesScale[pageIndex] || 1 - const factor = this.visualScale && this.scale ? (this.visualScale / this.scale) : 1 - return base * factor + return this.scale || 1 }, getPageComponent(docIndex, pageIndex) { const pageRef = this.$refs[`page${docIndex}-${pageIndex}`] @@ -772,9 +796,8 @@ export default defineComponent({ this.cachePageBounds() } } - if (this.autoFitZoom && !this.isAddingMode && !this.isDraggingElement) { + if (this.autoFitZoom && !this.isAddingMode && !this.isDraggingElement && !this.isPinching) { if (clientWidth && widthChanged) { - this.lastContainerWidth = clientWidth this.autoFitApplied = false this.scheduleAutoFitZoom() } @@ -898,25 +921,22 @@ export default defineComponent({ if (!event.ctrlKey) return event.preventDefault() + const currentScale = this.wheelZoomRafId ? this.pendingZoomScale : this.scale const factor = 1 - (event.deltaY * 0.002) - const nextVisual = this.clampZoomScale(this.visualScale * factor) - this.visualScale = nextVisual + this.pendingZoomScale = this.clampZoomScale(currentScale * factor) if (this.wheelZoomRafId) return this.wheelZoomRafId = window.requestAnimationFrame(() => { this.wheelZoomRafId = null - this.commitZoom() + this.commitZoom(this.pendingZoomScale) }) }, - commitZoom() { - const newScale = this.visualScale - + commitZoom(nextScale = this.pendingZoomScale) { + const newScale = this.clampZoomScale(nextScale || this.scale || 1) + this.isSyncingScale = true this.scale = newScale - - applyScaleToDocs(this.pdfDocuments, this.scale) - - this._pageMeasurementCache = markRaw({}) - this.cachePageBounds() + this.isSyncingScale = false + this.syncScaleState(newScale) }, finishAdding(event) { @@ -1059,14 +1079,14 @@ export default defineComponent({ if (!pageRef) return const measurement = this.getCachedMeasurement(docIndex, pageIndex, pageRef) const normalizedCanvasHeight = measurement.height - const pagesScale = doc.pagesScale[pageIndex] || 1 + const pageScale = this.scale || 1 pageObjects.forEach(object => { result.push({ ...object, pageIndex, pageNumber: pageIndex + 1, - scale: pagesScale, + scale: pageScale, normalizedCoordinates: { llx: Math.round(object.x), lly: Math.round(normalizedCanvasHeight - object.y), @@ -1327,7 +1347,6 @@ export default defineComponent({ onMeasure(e, docIndex, pageIndex) { if (docIndex < 0 || docIndex >= this.pdfDocuments.length) return - this.pdfDocuments[docIndex].pagesScale.splice(pageIndex, 1, e.scale) this._pageMeasurementCache[`${docIndex}-${pageIndex}`] = null this.cachePageBoundsForPage(docIndex, pageIndex) if (this.autoFitZoom) { @@ -1360,9 +1379,7 @@ export default defineComponent({ }, getCachedMeasurement(docIndex, pageIndex, pageRef) { const cacheKey = `${docIndex}-${pageIndex}` - const doc = this.pdfDocuments[docIndex] - const pagesScale = doc.pagesScale[pageIndex] || 1 - return getCachedMeasurement(this._pageMeasurementCache, cacheKey, pageRef, pagesScale) + return getCachedMeasurement(this._pageMeasurementCache, cacheKey, pageRef, this.scale || 1) }, calculateOptimalScale(maxPageWidth) { const containerWidth = this.$el?.clientWidth || 0 @@ -1379,8 +1396,8 @@ export default defineComponent({ this.adjustZoomToFit() }) }, - adjustZoomToFit() { - if (!this.autoFitZoom || this.autoFitApplied || !this.pdfDocuments.length) return + adjustZoomToFit(force = false) { + if ((!this.autoFitZoom && !force) || this.autoFitApplied || !this.pdfDocuments.length) return const widths = this.pdfDocuments .flatMap(doc => doc.pageWidths || []) @@ -1404,11 +1421,7 @@ export default defineComponent({ const optimalScale = this.calculateOptimalScale(maxCanvasWidth) this.autoFitApplied = true if (Math.abs(optimalScale - this.scale) > 0.01) { - this.scale = optimalScale - this.visualScale = optimalScale - applyScaleToDocs(this.pdfDocuments, this.scale) - this._pageMeasurementCache = markRaw({}) - this.cachePageBounds() + this.commitZoom(optimalScale) } }, }, From 14bc6511a324dac142ad17126415da1054cfc552 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 21 May 2026 15:00:54 -0300 Subject: [PATCH 3/3] test: add regression tests for zoom state sync and touch resize Covers the bugs fixed in the previous commit: - rename visualScale references to pendingZoomScale - wheel zoom accumulates correctly and pagesScale stays in sync - direct scale prop mutation triggers syncScaleState via watcher - out-of-range scale value is clamped to supported bounds - auto-fit runs on touch devices when container width changes - auto-fit is suppressed while isDraggingElement is true - auto-fit is suppressed while isPinching is true; resumes after - rotation-like resize (portrait->landscape) recomputes optimal scale - auto-fit is not triggered when width is unchanged (negative control) - forced one-shot fit works even when autoFitZoom prop is disabled Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/components/PDFElements.spec.ts | 269 ++++++++++++++++++++++++++- 1 file changed, 260 insertions(+), 9 deletions(-) diff --git a/tests/components/PDFElements.spec.ts b/tests/components/PDFElements.spec.ts index 707ca54..3866ef7 100644 --- a/tests/components/PDFElements.spec.ts +++ b/tests/components/PDFElements.spec.ts @@ -568,7 +568,7 @@ describe('PDFElements business rules', () => { ctx.pdfDocuments = [doc] await wrapper.setProps({ autoFitZoom: true }) ctx.scale = 1 - ctx.visualScale = 1 + ctx.pendingZoomScale = 1 Object.defineProperty(wrapper.element, 'clientWidth', { value: 500, @@ -579,7 +579,28 @@ describe('PDFElements business rules', () => { expect(ctx.autoFitApplied).toBe(true) expect(ctx.scale).toBe(2) - expect(ctx.visualScale).toBe(2) + expect(ctx.pendingZoomScale).toBe(2) + expect(ctx.pdfDocuments[0].pagesScale).toEqual([2, 2]) + }) + + it('allows forced one-shot fit when auto-fit is disabled', () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + doc.pageWidths = [100, 200] + ctx.pdfDocuments = [doc] + ctx.scale = 1 + ctx.pendingZoomScale = 1 + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 500, + configurable: true, + }) + + ctx.adjustZoomToFit(true) + + expect(ctx.autoFitApplied).toBe(true) + expect(ctx.scale).toBe(2) + expect(ctx.pendingZoomScale).toBe(2) expect(ctx.pdfDocuments[0].pagesScale).toEqual([2, 2]) }) @@ -587,7 +608,7 @@ describe('PDFElements business rules', () => { const { ctx } = makeWrapper() const doc = makeDoc() ctx.pdfDocuments = [doc] - ctx.visualScale = 1.6 + ctx.pendingZoomScale = 1.6 ctx.commitZoom() @@ -631,8 +652,7 @@ describe('PDFElements business rules', () => { doc.pageWidths = [100, 100] doc.pagesScale = [1, 1] ctx.pdfDocuments = [doc] - ctx.scale = 1 - ctx.visualScale = 1.5 + ctx.scale = 1.5 ctx.getPageCanvasElement = vi.fn(() => ({ width: 0, @@ -652,18 +672,249 @@ describe('PDFElements business rules', () => { expect(scale).toBe(1.5) }) - it('prefers per-page scale when no rect width is available', () => { + it('falls back to the current component scale when no rect width is available', () => { const { ctx } = makeWrapper() const doc = makeDoc() doc.pagesScale = [1.2, 1.4] ctx.pdfDocuments = [doc] - ctx.scale = 1 - ctx.visualScale = 1 + ctx.scale = 1.25 ctx.getPageCanvasElement = vi.fn(() => null) const scale = ctx.getDisplayedPageScale(0, 1) - expect(scale).toBe(1.4) + expect(scale).toBe(1.25) + }) + + it('accumulates wheel zoom using scale as the source of truth', () => { + const { ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + ctx.scale = 1 + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + + ctx.handleWheel({ ctrlKey: true, deltaY: -100, preventDefault: vi.fn() }) + + expect(ctx.scale).toBeGreaterThan(1) + expect(ctx.pendingZoomScale).toBe(ctx.scale) + expect(ctx.pdfDocuments[0].pagesScale).toEqual([ctx.scale, ctx.scale]) + + vi.unstubAllGlobals() + }) + + it('keeps page scales in sync when scale is mutated directly', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + ctx.scale = 1.6 + + await wrapper.vm.$nextTick() + + expect(ctx.pendingZoomScale).toBe(1.6) + expect(doc.pagesScale).toEqual([1.6, 1.6]) + }) + + it('clamps direct scale mutations to supported bounds', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + ctx.scale = 10 + + await wrapper.vm.$nextTick() + + expect(ctx.scale).toBe(3) + expect(ctx.pendingZoomScale).toBe(3) + expect(doc.pagesScale).toEqual([3, 3]) + }) + + it('re-applies auto-fit on touch devices when container width changes', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + await wrapper.setProps({ autoFitZoom: true }) + ctx.autoFitApplied = true + ctx.isTouchDevice = true + ctx.lastClientWidth = 320 + ctx.lastScrollTop = 0 + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 480, + configurable: true, + }) + Object.defineProperty(wrapper.element, 'scrollTop', { + value: 0, + configurable: true, + }) + + const scheduleSpy = vi.spyOn(ctx, 'scheduleAutoFitZoom').mockImplementation(() => {}) + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + + ctx.onViewportScroll() + + expect(ctx.autoFitApplied).toBe(false) + expect(scheduleSpy).toHaveBeenCalledTimes(1) + + vi.unstubAllGlobals() + }) + + it('does not re-apply auto-fit while dragging on touch resize', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + await wrapper.setProps({ autoFitZoom: true }) + ctx.isTouchDevice = true + ctx.isDraggingElement = true + ctx.lastClientWidth = 320 + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 480, + configurable: true, + }) + + const scheduleSpy = vi.spyOn(ctx, 'scheduleAutoFitZoom') + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + + ctx.onViewportScroll() + + expect(scheduleSpy).not.toHaveBeenCalled() + + vi.unstubAllGlobals() + }) + + it('recomputes scale after touch viewport rotation-like resize', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + doc.pageWidths = [100, 200] + ctx.pdfDocuments = [doc] + ctx.scale = 1 + ctx.pendingZoomScale = 1 + ctx.autoFitApplied = true + ctx.isTouchDevice = true + ctx.lastClientWidth = 320 + ctx.lastScrollTop = 0 + await wrapper.setProps({ autoFitZoom: true }) + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 500, + configurable: true, + }) + Object.defineProperty(wrapper.element, 'scrollTop', { + value: 0, + configurable: true, + }) + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + + ctx.onViewportScroll() + + expect(ctx.autoFitApplied).toBe(true) + expect(ctx.scale).toBe(2) + expect(ctx.pendingZoomScale).toBe(2) + expect(ctx.pdfDocuments[0].pagesScale).toEqual([2, 2]) + + vi.unstubAllGlobals() + }) + + it('does not trigger touch auto-fit when width is unchanged', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + ctx.pdfDocuments = [doc] + ctx.autoFitApplied = true + ctx.isTouchDevice = true + ctx.lastClientWidth = 480 + ctx.lastScrollTop = 0 + await wrapper.setProps({ autoFitZoom: true }) + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 480, + configurable: true, + }) + Object.defineProperty(wrapper.element, 'scrollTop', { + value: 0, + configurable: true, + }) + + const scheduleSpy = vi.spyOn(ctx, 'scheduleAutoFitZoom') + + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + + ctx.onViewportScroll() + + expect(scheduleSpy).not.toHaveBeenCalled() + expect(ctx.autoFitApplied).toBe(true) + + vi.unstubAllGlobals() + }) + + it('suppresses auto-fit during pinch and resumes after pinch ends', async () => { + const { wrapper, ctx } = makeWrapper() + const doc = makeDoc() + doc.pageWidths = [100, 200] + ctx.pdfDocuments = [doc] + ctx.scale = 1 + ctx.pendingZoomScale = 1 + ctx.autoFitApplied = true + ctx.isTouchDevice = true + ctx.lastClientWidth = 320 + ctx.lastScrollTop = 0 + await wrapper.setProps({ autoFitZoom: true }) + + const scheduleSpy = vi.spyOn(ctx, 'scheduleAutoFitZoom').mockImplementation(() => {}) + let rafCallback: FrameRequestCallback | null = null + const rafSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback: FrameRequestCallback) => { + rafCallback = callback + return 1 + }) + + ctx.isPinching = true + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 500, + configurable: true, + }) + Object.defineProperty(wrapper.element, 'scrollTop', { + value: 0, + configurable: true, + }) + + ctx.onViewportScroll() + rafCallback?.(0) + + expect(scheduleSpy).not.toHaveBeenCalled() + expect(ctx.autoFitApplied).toBe(true) + expect(ctx.scale).toBe(1) + + ctx.isPinching = false + ctx.autoFitApplied = true + ctx.lastClientWidth = 500 + + Object.defineProperty(wrapper.element, 'clientWidth', { + value: 640, + configurable: true, + }) + + ctx.onViewportScroll() + rafCallback?.(0) + + expect(scheduleSpy).toHaveBeenCalledTimes(1) + + rafSpy.mockRestore() }) })