fix(virtual-scroll): make horizontal scrolling work in RTL

Author: mmalerba

src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts

@@ -125,7 +125,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   attach(viewport: CdkVirtualScrollViewport) {
     this._averager.reset();
     this._viewport = viewport;
-    this._setScrollOffset();
+    this._renderContentForCurrentOffset();
   }
 
   /** Detaches this scroll strategy from the currently attached viewport. */
@@ -143,7 +143,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   /** @docs-private Implemented as part of VirtualScrollStrategy. */
   onDataLengthChanged() {
     if (this._viewport) {
-      this._setScrollOffset();
+      this._renderContentForCurrentOffset();
       this._checkRenderedContentSize();
     }
   }
@@ -241,7 +241,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
       // jitteriness if we just jump to the estimated position. Instead we make sure to scroll by
       // the same number of pixels as the scroll magnitude.
       if (scrollMagnitude >= viewport.getViewportSize()) {
-        this._setScrollOffset();
+        this._renderContentForCurrentOffset();
       } else {
         // The number of new items to render on the side the user is scrolling towards. Rather than
         // just filling the underscan space, we actually fill enough to have a buffer size of
@@ -346,18 +346,12 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
   }
 
   /**
-   * Sets the scroll offset and renders the content we estimate should be shown at that point.
-   * @param scrollOffset The offset to jump to. If not specified the scroll offset will not be
-   *     changed, but the rendered content will be recalculated based on our estimate of what should
-   *     be shown at the current scroll offset.
+   * Recalculates the rendered content based on our estimate of what should be shown at the current
+   * scroll offset.
    */
-  private _setScrollOffset(scrollOffset?: number) {
+  private _renderContentForCurrentOffset() {
     const viewport = this._viewport!;
-    if (scrollOffset == null) {
-      scrollOffset = viewport.measureScrollOffset();
-    } else {
-      viewport.setScrollOffset(scrollOffset);
-    }
+    const scrollOffset = viewport.measureScrollOffset();
     this._lastScrollOffset = scrollOffset;
     this._removalFailures = 0;
 

src/cdk/scrolling/scrolling-module.ts

@@ -15,7 +15,7 @@ import {CdkVirtualForOf} from './virtual-for-of';
 import {CdkVirtualScrollViewport} from './virtual-scroll-viewport';
 
 @NgModule({
-  imports: [PlatformModule, BidiModule],
+  imports: [BidiModule, PlatformModule],
   exports: [
     BidiModule,
     CdkFixedSizeVirtualScroll,

src/cdk/scrolling/virtual-scroll-viewport.scss

@@ -35,6 +35,7 @@ cdk-virtual-scroll-viewport {
   contain: strict;
   transform: translateZ(0);
   will-change: scroll-position;
+  -webkit-overflow-scrolling: touch;
 }
 
 // Wrapper element for the rendered content. This element will be transformed to push the rendered
@@ -45,6 +46,11 @@ cdk-virtual-scroll-viewport {
   left: 0;
   contain: content;
   will-change: transform;
+
+  [dir='rtl'] & {
+    right: 0;
+    left: auto;
+  }
 }
 
 .cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper {

src/cdk/scrolling/virtual-scroll-viewport.spec.ts

@@ -142,20 +142,6 @@ describe('CdkVirtualScrollViewport', () => {
       expect(viewport.getOffsetToRenderedContentStart()).toBe(10);
     }));
 
-    it('should set scroll offset', fakeAsync(() => {
-      finishInit(fixture);
-      viewport.setScrollOffset(testComponent.itemSize * 2);
-      fixture.detectChanges();
-      flush();
-
-      triggerScroll(viewport);
-      fixture.detectChanges();
-      flush();
-
-      expect(viewport.elementRef.nativeElement.scrollTop).toBe(testComponent.itemSize * 2);
-      expect(viewport.getRenderedRange()).toEqual({start: 2, end: 6});
-    }));
-
     it('should scroll to offset', fakeAsync(() => {
       finishInit(fixture);
       viewport.scrollToOffset(testComponent.itemSize * 2);
@@ -585,6 +571,92 @@ describe('CdkVirtualScrollViewport', () => {
       expect(() => finishInit(fixture)).toThrow();
     }));
   });
+
+  describe('with RTL direction', () => {
+    let fixture: ComponentFixture<FixedSizeVirtualScrollWithRtlDirection>;
+    let testComponent: FixedSizeVirtualScrollWithRtlDirection;
+    let viewport: CdkVirtualScrollViewport;
+    let viewportEl: HTMLElement;
+    let contentWrapperEl: HTMLElement;
+
+    beforeEach(() => {
+      TestBed.configureTestingModule({
+        imports: [ScrollingModule],
+        declarations: [FixedSizeVirtualScrollWithRtlDirection],
+      }).compileComponents();
+
+      fixture = TestBed.createComponent(FixedSizeVirtualScrollWithRtlDirection);
+      testComponent = fixture.componentInstance;
+      viewport = testComponent.viewport;
+      viewportEl = viewport.elementRef.nativeElement;
+      contentWrapperEl =
+          viewportEl.querySelector('.cdk-virtual-scroll-content-wrapper') as HTMLElement;
+    });
+
+    it('should initially be scrolled all the way right and showing the first item in horizontal' +
+       ' mode', fakeAsync(() => {
+          testComponent.orientation = 'horizontal';
+          finishInit(fixture);
+
+          expect(viewportEl.scrollLeft).toBe(
+              testComponent.itemSize * testComponent.items.length - testComponent.viewportSize);
+          expect(contentWrapperEl.style.transform).toMatch(/translateX\(0(px)?\)/);
+          expect((contentWrapperEl.children[0] as HTMLElement).innerText).toBe('0 - 0');
+        }));
+
+    it('should scroll through items as user scrolls to the left in horizontal mode',
+        fakeAsync(() => {
+          testComponent.orientation = 'horizontal';
+          finishInit(fixture);
+
+          triggerScroll(viewport, 0);
+          fixture.detectChanges();
+          flush();
+
+          expect(contentWrapperEl.style.transform).toBe('translateX(-300px)');
+          expect((contentWrapperEl.children[0] as HTMLElement).innerText).toBe('6 - 6');
+        }));
+
+    it('should interpret scrollToOffset amount as an offset from the right in horizontal mode',
+        fakeAsync(() => {
+          testComponent.orientation = 'horizontal';
+          finishInit(fixture);
+
+          viewport.scrollToOffset(100);
+          triggerScroll(viewport);
+          fixture.detectChanges();
+          flush();
+
+          expect(viewportEl.scrollLeft).toBe(testComponent.itemSize * testComponent.items.length -
+              testComponent.viewportSize - 100);
+        }));
+
+    it('should scroll to the correct index in horizontal mode', fakeAsync(() => {
+      testComponent.orientation = 'horizontal';
+      finishInit(fixture);
+
+      viewport.scrollToIndex(2);
+      triggerScroll(viewport);
+      fixture.detectChanges();
+      flush();
+
+      expect((contentWrapperEl.children[0] as HTMLElement).innerText).toBe('2 - 2');
+    }));
+
+    it('should emit the scrolled to index in horizontal mode', fakeAsync(() => {
+      testComponent.orientation = 'horizontal';
+      finishInit(fixture);
+
+      expect(testComponent.scrolledToIndex).toBe(0);
+
+      viewport.scrollToIndex(2);
+      triggerScroll(viewport);
+      fixture.detectChanges();
+      flush();
+
+      expect(testComponent.scrolledToIndex).toBe(2);
+    }));
+  });
 });
 
 
@@ -597,6 +669,11 @@ function finishInit(fixture: ComponentFixture<any>) {
   // On the second cycle we render the items.
   fixture.detectChanges();
   flush();
+
+  // Flush the initial fake scroll event.
+  animationFrameScheduler.flush();
+  flush();
+  fixture.detectChanges();
 }
 
 /** Trigger a scroll event on the viewport (optionally setting a new scroll offset). */
@@ -663,3 +740,53 @@ class FixedSizeVirtualScroll {
     return this.orientation == 'horizontal' ? this.viewportCrossSize : this.viewportSize;
   }
 }
+
+@Component({
+  template: `
+    <cdk-virtual-scroll-viewport dir="rtl"
+        [itemSize]="itemSize" [bufferSize]="bufferSize" [orientation]="orientation"
+        [style.height.px]="viewportHeight" [style.width.px]="viewportWidth"
+        (scrolledIndexChange)="scrolledToIndex = $event">
+      <div class="item"
+           *cdkVirtualFor="let item of items; let i = index; trackBy: trackBy; \
+                           templateCacheSize: templateCacheSize"
+           [style.height.px]="itemSize" [style.width.px]="itemSize">
+        {{i}} - {{item}}
+      </div>
+    </cdk-virtual-scroll-viewport>
+  `,
+  styles: [`
+    .cdk-virtual-scroll-content-wrapper {
+      display: flex;
+      flex-direction: column;
+    }
+
+    .cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper {
+      flex-direction: row;
+    }
+  `],
+  encapsulation: ViewEncapsulation.None,
+})
+class FixedSizeVirtualScrollWithRtlDirection {
+  @ViewChild(CdkVirtualScrollViewport) viewport: CdkVirtualScrollViewport;
+  @ViewChild(CdkVirtualForOf, {read: ViewContainerRef}) virtualForViewContainer: ViewContainerRef;
+
+  @Input() orientation = 'vertical';
+  @Input() viewportSize = 200;
+  @Input() viewportCrossSize = 100;
+  @Input() itemSize = 50;
+  @Input() bufferSize = 0;
+  @Input() items = Array(10).fill(0).map((_, i) => i);
+  @Input() trackBy;
+  @Input() templateCacheSize = 20;
+
+  scrolledToIndex = 0;
+
+  get viewportWidth() {
+    return this.orientation == 'horizontal' ? this.viewportSize : this.viewportCrossSize;
+  }
+
+  get viewportHeight() {
+    return this.orientation == 'horizontal' ? this.viewportCrossSize : this.viewportSize;
+  }
+}

src/cdk/scrolling/virtual-scroll-viewport.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {Directionality} from '@angular/cdk/bidi';
 import {ListRange} from '@angular/cdk/collections';
 import {supportsScrollBehavior} from '@angular/cdk/platform';
 import {
@@ -18,12 +19,13 @@ import {
   NgZone,
   OnDestroy,
   OnInit,
+  Optional,
   Output,
   ViewChild,
   ViewEncapsulation,
 } from '@angular/core';
 import {animationFrameScheduler, fromEvent, Observable, Subject} from 'rxjs';
-import {sampleTime, takeUntil} from 'rxjs/operators';
+import {sampleTime, startWith, takeUntil} from 'rxjs/operators';
 import {CdkVirtualForOf} from './virtual-for-of';
 import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy';
 
@@ -99,9 +101,6 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
   /** The size of the viewport (in pixels). */
   private _viewportSize = 0;
 
-  /** The pending scroll offset to be applied during the next change detection cycle. */
-  private _pendingScrollOffset: number | null;
-
   /** the currently attached CdkVirtualForOf. */
   private _forOf: CdkVirtualForOf<any> | null;
 
@@ -126,7 +125,8 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
   constructor(public elementRef: ElementRef<HTMLElement>,
               private _changeDetectorRef: ChangeDetectorRef,
               private _ngZone: NgZone,
-              @Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}
+              @Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy,
+              @Optional() private _dir: Directionality) {}
 
   ngOnInit() {
     // It's still too early to measure the viewport at this point. Deferring with a promise allows
@@ -138,9 +138,13 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
       this._scrollStrategy.attach(this);
 
       fromEvent(this.elementRef.nativeElement, 'scroll')
-          // Sample the scroll stream at every animation frame. This way if there are multiple
-          // scroll events in the same frame we only need to recheck our layout once.
-          .pipe(sampleTime(0, animationFrameScheduler), takeUntil(this._destroyed))
+          .pipe(
+              // Start off with a fake scroll event so we properly detect our initial position.
+              startWith(null!),
+              // Sample the scroll stream at every animation frame. This way if there are multiple
+              // scroll events in the same frame we only need to recheck our layout once.
+              sampleTime(0, animationFrameScheduler),
+              takeUntil(this._destroyed))
           .subscribe(() => this._scrollStrategy.onContentScrolled());
 
       this._markChangeDetectionNeeded();
@@ -238,8 +242,12 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
    * (in pixels).
    */
   setRenderedContentOffset(offset: number, to: 'to-start' | 'to-end' = 'to-start') {
+    // For a horizontal viewport in a right-to-left language we need to translate along the x-axis
+    // in the negative direction.
+    const axisDirection = this.orientation == 'horizontal' && this._dir && this._dir.value == 'rtl'
+        ? -1 : 1;
     const axis = this.orientation === 'horizontal' ? 'X' : 'Y';
-    let transform = `translate${axis}(${Number(offset)}px)`;
+    let transform = `translate${axis}(${Number(axisDirection * offset)}px)`;
     this._renderedContentOffset = offset;
     if (to === 'to-end') {
       transform += ` translate${axis}(-100%)`;
@@ -265,13 +273,20 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
   }
 
   /**
-   * Scrolls to the offset on the viewport.
+   * Scrolls to the given offset from the start of the viewport. Please note that this is not always
+   * the same as setting `scrollTop` or `scrollLeft`. In a horizontal viewport with right-to-left
+   * direction, this would be the equivalent of setting a fictional `scrollRight` property.
    * @param offset The offset to scroll to.
    * @param behavior The ScrollBehavior to use when scrolling. Default is behavior is `auto`.
    */
   scrollToOffset(offset: number, behavior: ScrollBehavior = 'auto') {
     const viewportElement = this.elementRef.nativeElement;
 
+    // For a horizontal viewport in a right-to-left language we need to calculate what `scrollRight`
+    // would be.
+    offset = this.orientation == 'horizontal' && this._dir && this._dir.value == 'rtl' ?
+        Math.max(0, this._totalContentSize - this._viewportSize - offset) : offset;
+
     if (supportsScrollBehavior()) {
       const offsetDirection = this.orientation === 'horizontal' ? 'left' : 'top';
       viewportElement.scrollTo({[offsetDirection]: offset, behavior});
@@ -293,18 +308,16 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
     this._scrollStrategy.scrollToIndex(index, behavior);
   }
 
-  /** @docs-private Internal method to set the scroll offset on the viewport. */
-  setScrollOffset(offset: number) {
-    // Rather than setting the offset immediately, we batch it up to be applied along with other DOM
-    // writes during the next change detection cycle.
-    this._pendingScrollOffset = offset;
-    this._markChangeDetectionNeeded();
-  }
-
-  /** Gets the current scroll offset of the viewport (in pixels). */
+  /** Gets the current scroll offset from the start of the viewport (in pixels). */
   measureScrollOffset(): number {
-    return this.orientation === 'horizontal' ?
-        this.elementRef.nativeElement.scrollLeft : this.elementRef.nativeElement.scrollTop;
+    if (this.orientation == 'horizontal') {
+      const offset = this.elementRef.nativeElement.scrollLeft;
+      // For a horizontal viewport in a right-to-left language we need to calculate what
+      // `scrollRight` would be.
+      return this._dir && this._dir.value == 'rtl' ?
+          Math.max(0, this._totalContentSize - this._viewportSize - offset) : offset;
+    }
+    return this.elementRef.nativeElement.scrollTop;
   }
 
   /** Measure the combined size of all of the rendered items. */
@@ -367,14 +380,6 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
     // string literals, a variable that can only be 'X' or 'Y', and user input that is run through
     // the `Number` function first to coerce it to a numeric value.
     this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
-    // Apply the pending scroll offset separately, since it can't be set up as an Angular binding.
-    if (this._pendingScrollOffset != null) {
-      if (this.orientation === 'horizontal') {
-        this.elementRef.nativeElement.scrollLeft = this._pendingScrollOffset;
-      } else {
-        this.elementRef.nativeElement.scrollTop = this._pendingScrollOffset;
-      }
-    }
 
     const runAfterChangeDetection = this._runAfterChangeDetection;
     this._runAfterChangeDetection = [];