fix(plugin-scroll): horizontal layout extent, centering and programmatic scroll

BREAKING CHANGE: none

- Use scroll-axis extent (width vs height) for visible range and end spacing
  so horizontal mode no longer treats item height as the scroll extent.
- Disable horizontal centering (getCenteringOffsetX → 0) so pages stay in a
  plain row; vertical keeps centering via total content width.
- During pageChangeState.isChanging, keep store currentPage from viewport-
  derived metrics and avoid duplicate pageChange$ until user-driven scroll
  resumes; startPageChange sets currentPage + emits pageChange$ optimistically.
- In commitMetrics, dispatch before scroll$.emit so listeners that read state
  see metricsToCommit (single source of truth).
- Set PageLayout.elevated: false when building virtual items (required field;
  selectors still override from interaction manager).

Author: luissardon

packages/plugin-scroll/src/lib/scroll-plugin.ts

@@ -435,7 +435,18 @@ export class ScrollPlugin extends BasePlugin<
       startTime: Date.now(),
     };
 
-    this.dispatch(updateDocumentScrollState(documentId, { pageChangeState }));
+    // Optimistic update: set currentPage immediately so UI reflects target during smooth scroll
+    this.dispatch(
+      updateDocumentScrollState(documentId, {
+        pageChangeState,
+        currentPage: targetPage,
+      }),
+    );
+    this.pageChange$.emit({
+      documentId,
+      pageNumber: targetPage,
+      totalPages: docState.totalPages,
+    });
 
     if (behavior === 'instant') {
       this.completePageChange(documentId);
@@ -494,14 +505,22 @@ export class ScrollPlugin extends BasePlugin<
     const docState = this.getDocumentState(documentId);
     if (!docState) return;
 
-    // Update state
-    this.dispatch(updateDocumentScrollState(documentId, metrics));
-
-    // Emit scroll event
-    this.scroll$.emit({ documentId, metrics });
-
-    // Emit page change if current page changed
-    if (metrics.currentPage !== docState.currentPage) {
+    // During smooth scroll (Next/Previous), viewport fires before scroll completes.
+    // Metrics would derive currentPage from old viewport position and revert the optimistic update.
+    const isProgrammaticScroll = docState.pageChangeState.isChanging;
+    // Preserve optimistic currentPage during programmatic scroll; use viewport-derived value otherwise
+    const metricsToCommit =
+      isProgrammaticScroll && metrics.currentPage !== docState.currentPage
+        ? { ...metrics, currentPage: docState.currentPage }
+        : metrics;
+
+    // Update state before emitting scroll event,
+    // so any scroll$ listener that reads state sees metricsToCommit; then notify
+    this.dispatch(updateDocumentScrollState(documentId, metricsToCommit));
+    this.scroll$.emit({ documentId, metrics: metricsToCommit });
+
+    // Only emit pageChange$ for user-driven scroll; programmatic scroll already emitted in startPageChange
+    if (!isProgrammaticScroll && metrics.currentPage !== docState.currentPage) {
       this.pageChange$.emit({
         documentId,
         pageNumber: metrics.currentPage,

packages/plugin-scroll/src/lib/strategies/base-strategy.ts

@@ -33,6 +33,22 @@ export abstract class BaseScrollStrategy {
   abstract getTotalContentSize(virtualItems: VirtualItem[]): Size;
   protected abstract getScrollOffset(viewport: ViewportMetrics): number;
   protected abstract getClientSize(viewport: ViewportMetrics): number;
+  
+  /**
+   * Returns the item's extent along the scroll axis. Vertical layout uses height;
+   * horizontal layout uses width. Used for visible range and end spacing calculations.
+   */
+  protected abstract getItemSizeAlongScrollAxis(item: VirtualItem): number;
+
+  /**
+   * Horizontal centering offset for items narrower than the content max width.
+   * Vertical layout centers spreads within the row; horizontal layout overrides to 0
+   * since items stay in a simple row without centering.
+   */
+  protected getCenteringOffsetX(_item: VirtualItem, totalContentSize: Size | undefined): number {
+    if (!totalContentSize || _item.width >= totalContentSize.width) return 0;
+    return (totalContentSize.width - _item.width) / 2;
+  }
 
   protected getVisibleRange(
     viewport: ViewportMetrics,
@@ -44,10 +60,14 @@ export abstract class BaseScrollStrategy {
     const viewportStart = scrollOffset;
     const viewportEnd = scrollOffset + clientSize;
 
+    // Use extent along scroll axis (height for vertical, width for horizontal)
     let startIndex = 0;
     while (
       startIndex < virtualItems.length &&
-      (virtualItems[startIndex].offset + virtualItems[startIndex].height) * scale <= viewportStart
+      (virtualItems[startIndex].offset +
+        this.getItemSizeAlongScrollAxis(virtualItems[startIndex])) *
+        scale <=
+        viewportStart
     ) {
       startIndex++;
     }
@@ -75,7 +95,7 @@ export abstract class BaseScrollStrategy {
       visibleItems,
       viewport,
       scale,
-      totalContentSize.width,
+      totalContentSize,
     );
     const visiblePages = pageVisibilityMetrics.map((m) => m.pageNumber);
     const renderedPageIndexes = virtualItems
@@ -85,11 +105,11 @@ export abstract class BaseScrollStrategy {
     const first = virtualItems[range.start];
     const last = virtualItems[range.end];
     const startSpacing = first ? first.offset * scale : 0;
+    const lastItem = virtualItems[virtualItems.length - 1];
+    // End spacing = distance from last rendered item to end of content (uses extent along scroll axis)
     const endSpacing = last
-      ? (virtualItems[virtualItems.length - 1].offset + // end of content
-          virtualItems[virtualItems.length - 1].height) *
-          scale - // minus
-        (last.offset + last.height) * scale // end of last rendered
+      ? (lastItem.offset + this.getItemSizeAlongScrollAxis(lastItem)) * scale -
+        (last.offset + this.getItemSizeAlongScrollAxis(last)) * scale
       : 0;
 
     return {
@@ -107,15 +127,12 @@ export abstract class BaseScrollStrategy {
     virtualItems: VirtualItem[],
     viewport: ViewportMetrics,
     scale: number,
-    contentWidth?: number,
+    totalContentSize?: Size,
   ): ScrollMetrics['pageVisibilityMetrics'] {
     const visibilityMetrics: ScrollMetrics['pageVisibilityMetrics'] = [];
-    // Calculate max width for centering if not provided
-    const maxWidth = contentWidth ?? Math.max(...virtualItems.map((i) => i.width));
 
     virtualItems.forEach((item) => {
-      // Calculate horizontal centering offset for items narrower than max width
-      const centeringOffsetX = item.width < maxWidth ? (maxWidth - item.width) / 2 : 0;
+      const centeringOffsetX = this.getCenteringOffsetX(item, totalContentSize);
 
       item.pageLayouts.forEach((page) => {
         const itemX = (item.x + centeringOffsetX) * scale;
@@ -194,14 +211,7 @@ export abstract class BaseScrollStrategy {
     const pageLayout = item.pageLayouts.find((layout) => layout.pageNumber === pageNumber);
     if (!pageLayout) return null;
 
-    // Calculate centering offset for items that are narrower than the maximum width
-    let centeringOffsetX = 0;
-    if (totalContentSize) {
-      const maxWidth = totalContentSize.width;
-      if (item.width < maxWidth) {
-        centeringOffsetX = (maxWidth - item.width) / 2;
-      }
-    }
+    const centeringOffsetX = this.getCenteringOffsetX(item, totalContentSize);
 
     return {
       origin: {

packages/plugin-scroll/src/lib/strategies/horizontal-strategy.ts

@@ -1,4 +1,5 @@
 import { PdfPageObjectWithRotatedSize } from '@embedpdf/models';
+import { Size } from '@embedpdf/models';
 import { ViewportMetrics } from '@embedpdf/plugin-viewport';
 import { BaseScrollStrategy, ScrollStrategyConfig } from './base-strategy';
 import { VirtualItem, PageLayout } from '../types/virtual-item';
@@ -22,6 +23,7 @@ export class HorizontalScrollStrategy extends BaseScrollStrategy {
           height: page.size.height,
           rotatedWidth: page.rotatedSize.width,
           rotatedHeight: page.rotatedSize.height,
+          elevated: false,
         };
         pageX += page.rotatedSize.width + this.pageGap;
         return layout;
@@ -66,4 +68,18 @@ export class HorizontalScrollStrategy extends BaseScrollStrategy {
   protected getClientSize(viewport: ViewportMetrics): number {
     return viewport.clientWidth;
   }
+
+  /** Horizontal scroll: extent along scroll axis is width. */
+  protected getItemSizeAlongScrollAxis(item: VirtualItem): number {
+    return item.width;
+  }
+
+  /**
+   * No centering for horizontal layout. Items are laid out in a simple row;
+   * using total content width would produce incorrect offsets and shift pages.
+   */
+  /* eslint-disable no-unused-vars -- override intentionally ignores params */
+  protected getCenteringOffsetX(_item: VirtualItem, _totalContentSize: Size | undefined): number {
+    return 0;
+  }
 }

packages/plugin-scroll/src/lib/strategies/vertical-strategy.ts

@@ -2,7 +2,6 @@ import { PdfPageObjectWithRotatedSize } from '@embedpdf/models';
 import { ViewportMetrics } from '@embedpdf/plugin-viewport';
 import { BaseScrollStrategy, ScrollStrategyConfig } from './base-strategy';
 import { VirtualItem, PageLayout } from '../types/virtual-item';
-import { ScrollMetrics } from '../types';
 
 export class VerticalScrollStrategy extends BaseScrollStrategy {
   constructor(config: ScrollStrategyConfig) {
@@ -23,6 +22,7 @@ export class VerticalScrollStrategy extends BaseScrollStrategy {
           height: page.size.height,
           rotatedWidth: page.rotatedSize.width,
           rotatedHeight: page.rotatedSize.height,
+          elevated: false,
         };
         pageX += page.rotatedSize.width + this.pageGap;
         return layout;
@@ -67,4 +67,9 @@ export class VerticalScrollStrategy extends BaseScrollStrategy {
   protected getClientSize(viewport: ViewportMetrics): number {
     return viewport.clientHeight;
   }
+
+  /** Vertical scroll: extent along scroll axis is height. */
+  protected getItemSizeAlongScrollAxis(item: VirtualItem): number {
+    return item.height;
+  }
 }