Skip to content

Commit 22953a9

Browse files
mmalerbajelbourn
authored andcommitted
fix(virtual-scroll): correct horizontal scrolling in RTL (#12512)
1 parent 559b95e commit 22953a9

File tree

8 files changed

+313
-125
lines changed

8 files changed

+313
-125
lines changed

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
125125
attach(viewport: CdkVirtualScrollViewport) {
126126
this._averager.reset();
127127
this._viewport = viewport;
128-
this._setScrollOffset();
128+
this._renderContentForCurrentOffset();
129129
}
130130

131131
/** Detaches this scroll strategy from the currently attached viewport. */
@@ -143,7 +143,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
143143
/** @docs-private Implemented as part of VirtualScrollStrategy. */
144144
onDataLengthChanged() {
145145
if (this._viewport) {
146-
this._setScrollOffset();
146+
this._renderContentForCurrentOffset();
147147
this._checkRenderedContentSize();
148148
}
149149
}
@@ -241,7 +241,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
241241
// jitteriness if we just jump to the estimated position. Instead we make sure to scroll by
242242
// the same number of pixels as the scroll magnitude.
243243
if (scrollMagnitude >= viewport.getViewportSize()) {
244-
this._setScrollOffset();
244+
this._renderContentForCurrentOffset();
245245
} else {
246246
// The number of new items to render on the side the user is scrolling towards. Rather than
247247
// just filling the underscan space, we actually fill enough to have a buffer size of
@@ -346,18 +346,12 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
346346
}
347347

348348
/**
349-
* Sets the scroll offset and renders the content we estimate should be shown at that point.
350-
* @param scrollOffset The offset to jump to. If not specified the scroll offset will not be
351-
* changed, but the rendered content will be recalculated based on our estimate of what should
352-
* be shown at the current scroll offset.
349+
* Recalculates the rendered content based on our estimate of what should be shown at the current
350+
* scroll offset.
353351
*/
354-
private _setScrollOffset(scrollOffset?: number) {
352+
private _renderContentForCurrentOffset() {
355353
const viewport = this._viewport!;
356-
if (scrollOffset == null) {
357-
scrollOffset = viewport.measureScrollOffset();
358-
} else {
359-
viewport.setScrollOffset(scrollOffset);
360-
}
354+
const scrollOffset = viewport.measureScrollOffset();
361355
this._lastScrollOffset = scrollOffset;
362356
this._removalFailures = 0;
363357

src/cdk/scrolling/scrollable.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,21 @@ export class CdkScrollable implements OnInit, OnDestroy {
4848
private _destroyed = new Subject();
4949

5050
private _elementScrolled: Observable<Event> = Observable.create(observer =>
51-
this._ngZone.runOutsideAngular(() =>
52-
fromEvent(this._elementRef.nativeElement, 'scroll').pipe(takeUntil(this._destroyed))
51+
this.ngZone.runOutsideAngular(() =>
52+
fromEvent(this.elementRef.nativeElement, 'scroll').pipe(takeUntil(this._destroyed))
5353
.subscribe(observer)));
5454

55-
constructor(private _elementRef: ElementRef<HTMLElement>,
56-
private _scroll: ScrollDispatcher,
57-
private _ngZone: NgZone,
58-
@Optional() private _dir?: Directionality) {}
55+
constructor(protected elementRef: ElementRef<HTMLElement>,
56+
protected scrollDispatcher: ScrollDispatcher,
57+
protected ngZone: NgZone,
58+
@Optional() protected dir?: Directionality) {}
5959

6060
ngOnInit() {
61-
this._scroll.register(this);
61+
this.scrollDispatcher.register(this);
6262
}
6363

6464
ngOnDestroy() {
65-
this._scroll.deregister(this);
65+
this.scrollDispatcher.deregister(this);
6666
this._destroyed.next();
6767
this._destroyed.complete();
6868
}
@@ -74,7 +74,7 @@ export class CdkScrollable implements OnInit, OnDestroy {
7474

7575
/** Gets the ElementRef for the viewport. */
7676
getElementRef(): ElementRef<HTMLElement> {
77-
return this._elementRef;
77+
return this.elementRef;
7878
}
7979

8080
/**
@@ -86,8 +86,8 @@ export class CdkScrollable implements OnInit, OnDestroy {
8686
* @param options specified the offsets to scroll to.
8787
*/
8888
scrollTo(options: ExtendedScrollToOptions): void {
89-
const el = this._elementRef.nativeElement;
90-
const isRtl = this._dir && this._dir.value == 'rtl';
89+
const el = this.elementRef.nativeElement;
90+
const isRtl = this.dir && this.dir.value == 'rtl';
9191

9292
// Rewrite start & end offsets as right or left offsets.
9393
options.left = options.left == null ? (isRtl ? options.end : options.start) : options.left;
@@ -119,7 +119,7 @@ export class CdkScrollable implements OnInit, OnDestroy {
119119
}
120120

121121
private _applyScrollToOptions(options: ScrollToOptions): void {
122-
const el = this._elementRef.nativeElement;
122+
const el = this.elementRef.nativeElement;
123123

124124
if (supportsScrollBehavior()) {
125125
el.scrollTo(options);
@@ -145,7 +145,7 @@ export class CdkScrollable implements OnInit, OnDestroy {
145145
measureScrollOffset(from: 'top' | 'left' | 'right' | 'bottom' | 'start' | 'end'): number {
146146
const LEFT = 'left';
147147
const RIGHT = 'right';
148-
const el = this._elementRef.nativeElement;
148+
const el = this.elementRef.nativeElement;
149149
if (from == 'top') {
150150
return el.scrollTop;
151151
}
@@ -154,7 +154,7 @@ export class CdkScrollable implements OnInit, OnDestroy {
154154
}
155155

156156
// Rewrite start & end as left or right offsets.
157-
const isRtl = this._dir && this._dir.value == 'rtl';
157+
const isRtl = this.dir && this.dir.value == 'rtl';
158158
if (from == 'start') {
159159
from = isRtl ? RIGHT : LEFT;
160160
} else if (from == 'end') {

src/cdk/scrolling/scrolling-module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {CdkVirtualForOf} from './virtual-for-of';
1515
import {CdkVirtualScrollViewport} from './virtual-scroll-viewport';
1616

1717
@NgModule({
18-
imports: [PlatformModule, BidiModule],
18+
imports: [BidiModule, PlatformModule],
1919
exports: [
2020
BidiModule,
2121
CdkFixedSizeVirtualScroll,

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ cdk-virtual-scroll-viewport {
3535
contain: strict;
3636
transform: translateZ(0);
3737
will-change: scroll-position;
38+
-webkit-overflow-scrolling: touch;
3839
}
3940

4041
// Wrapper element for the rendered content. This element will be transformed to push the rendered
@@ -44,7 +45,14 @@ cdk-virtual-scroll-viewport {
4445
top: 0;
4546
left: 0;
4647
contain: content;
47-
will-change: transform;
48+
49+
// Note: We can't put `will-change: transform;` here because it causes Safari to not update the
50+
// viewport's `scrollHeight` when the spacer's transform changes.
51+
52+
[dir='rtl'] & {
53+
right: 0;
54+
left: auto;
55+
}
4856
}
4957

5058
.cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper {
@@ -67,7 +75,9 @@ cdk-virtual-scroll-viewport {
6775
height: 1px;
6876
width: 1px;
6977
transform-origin: 0 0;
70-
will-change: transform;
78+
79+
// Note: We can't put `will-change: transform;` here because it causes Safari to not update the
80+
// viewport's `scrollHeight` when the spacer's transform changes.
7181

7282
[dir='rtl'] & {
7383
right: 0;

0 commit comments

Comments
 (0)