Skip to content

feat(cdk/scrolling): make scroller element configurable for virtual scrolling #24394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
3 changes: 3 additions & 0 deletions src/cdk/scrolling/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ export * from './virtual-for-of';
export * from './virtual-scroll-strategy';
export * from './virtual-scroll-viewport';
export * from './virtual-scroll-repeater';
export * from './virtual-scrollable';
export * from './virtual-scrollable-element';
export * from './virtual-scrollable-window';
4 changes: 2 additions & 2 deletions src/cdk/scrolling/scrollable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export type ExtendedScrollToOptions = _XAxis & _YAxis & ScrollOptions;
selector: '[cdk-scrollable], [cdkScrollable]',
})
export class CdkScrollable implements OnInit, OnDestroy {
private readonly _destroyed = new Subject<void>();
protected readonly _destroyed = new Subject<void>();

private _elementScrolled: Observable<Event> = new Observable((observer: Observer<Event>) =>
protected _elementScrolled: Observable<Event> = new Observable((observer: Observer<Event>) =>
this.ngZone.runOutsideAngular(() =>
fromEvent(this.elementRef.nativeElement, 'scroll')
.pipe(takeUntil(this._destroyed))
Expand Down
12 changes: 11 additions & 1 deletion src/cdk/scrolling/scrolling-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {CdkFixedSizeVirtualScroll} from './fixed-size-virtual-scroll';
import {CdkScrollable} from './scrollable';
import {CdkVirtualForOf} from './virtual-for-of';
import {CdkVirtualScrollViewport} from './virtual-scroll-viewport';
import {CdkVirtualScrollableElement} from './virtual-scrollable-element';
import {CdkVirtualScrollableWindow} from './virtual-scrollable-window';

@NgModule({
exports: [CdkScrollable],
Expand All @@ -30,7 +32,15 @@ export class CdkScrollableModule {}
CdkFixedSizeVirtualScroll,
CdkVirtualForOf,
CdkVirtualScrollViewport,
CdkVirtualScrollableWindow,
CdkVirtualScrollableElement,
],
declarations: [
CdkFixedSizeVirtualScroll,
CdkVirtualForOf,
CdkVirtualScrollViewport,
CdkVirtualScrollableWindow,
CdkVirtualScrollableElement,
],
declarations: [CdkFixedSizeVirtualScroll, CdkVirtualForOf, CdkVirtualScrollViewport],
})
export class ScrollingModule {}
17 changes: 8 additions & 9 deletions src/cdk/scrolling/virtual-scroll-viewport.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@
}


// Scrolling container.
// viewport
cdk-virtual-scroll-viewport {
display: block;
position: relative;
overflow: auto;
contain: strict;
transform: translateZ(0);
}

// Scrolling container.
.cdk-virtual-scrollable {
overflow: auto;
will-change: scroll-position;
contain: strict;
-webkit-overflow-scrolling: touch;
}

Expand Down Expand Up @@ -69,19 +73,14 @@ cdk-virtual-scroll-viewport {
// set if it were rendered all at once. This ensures that the scrollable content region is the
// correct size.
.cdk-virtual-scroll-spacer {
position: absolute;
Copy link
Contributor

@mmalerba mmalerba Jun 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spike-rabbit I started looking into why some tests were failing internally with this change, and it looked like some people were setting display: flex on the viewport. Without this position: absolute the spacer is collapsing and throwing off the scrollbar.

I think we could definitely question why they're making it display: flex, as I don't see any real reason to do that, but I think we could help prevent unexpected breakages by just slapping a flex: 0 0 auto on here.

I also notice that below you still have right: 0 and left: auto which can be removed, now that its no longer position: absolute

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmalerba changed as you requested

top: 0;
left: 0;
height: 1px;
width: 1px;
transform-origin: 0 0;
flex: 0 0 auto; // prevents spacer from collapsing if display: flex is applied

// Note: We can't put `will-change: transform;` here because it causes Safari to not update the
// viewport's `scrollHeight` when the spacer's transform changes.

[dir='rtl'] & {
right: 0;
left: auto;
transform-origin: 100% 0;
}
}
161 changes: 158 additions & 3 deletions src/cdk/scrolling/virtual-scroll-viewport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,9 @@ describe('CdkVirtualScrollViewport', () => {
spyOn(dispatcher, 'register').and.callThrough();
spyOn(dispatcher, 'deregister').and.callThrough();
finishInit(fixture);
expect(dispatcher.register).toHaveBeenCalledWith(testComponent.viewport);
expect(dispatcher.register).toHaveBeenCalledWith(testComponent.viewport.scrollable);
fixture.destroy();
expect(dispatcher.deregister).toHaveBeenCalledWith(testComponent.viewport);
expect(dispatcher.deregister).toHaveBeenCalledWith(testComponent.viewport.scrollable);
}),
));

Expand Down Expand Up @@ -1086,6 +1086,76 @@ describe('CdkVirtualScrollViewport', () => {
expect(viewport.getOffsetToRenderedContentStart()).toBe(0);
}));
});

describe('with custom scrolling element', () => {
let fixture: ComponentFixture<VirtualScrollWithCustomScrollingElement>;
let testComponent: VirtualScrollWithCustomScrollingElement;
let viewport: CdkVirtualScrollViewport;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [ScrollingModule],
declarations: [VirtualScrollWithCustomScrollingElement],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(VirtualScrollWithCustomScrollingElement);
testComponent = fixture.componentInstance;
viewport = testComponent.viewport;
});

it('should measure viewport offset', fakeAsync(() => {
finishInit(fixture);

expect(viewport.measureViewportOffset('top'))
.withContext('with scrolling-element padding-top: 50 offset should be 50')
.toBe(50);
}));

it('should measure scroll offset', fakeAsync(() => {
finishInit(fixture);
triggerScroll(viewport, 100);
fixture.detectChanges();
flush();

expect(viewport.measureScrollOffset('top'))
.withContext('should be 50 (actual scroll offset - viewport offset)')
.toBe(50);
}));
});

describe('with scrollable window', () => {
let fixture: ComponentFixture<VirtualScrollWithScrollableWindow>;
let testComponent: VirtualScrollWithScrollableWindow;
let viewport: CdkVirtualScrollViewport;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [ScrollingModule],
declarations: [VirtualScrollWithScrollableWindow],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(VirtualScrollWithScrollableWindow);
testComponent = fixture.componentInstance;
viewport = testComponent.viewport;
});

it('should measure scroll offset', fakeAsync(() => {
finishInit(fixture);
viewport.scrollToOffset(100 + 8); // the +8 is due to a horizontal scrollbar
dispatchFakeEvent(window, 'scroll', true);
tick();
fixture.detectChanges();
flush();

expect(viewport.measureScrollOffset('top'))
.withContext('should be 50 (actual scroll offset - viewport offset)')
.toBe(50);
}));
});
});

/** Finish initializing the virtual scroll component at the beginning of a test. */
Expand All @@ -1109,7 +1179,7 @@ function triggerScroll(viewport: CdkVirtualScrollViewport, offset?: number) {
if (offset !== undefined) {
viewport.scrollToOffset(offset);
}
dispatchFakeEvent(viewport.elementRef.nativeElement, 'scroll');
dispatchFakeEvent(viewport.scrollable.getElementRef().nativeElement, 'scroll');
animationFrameScheduler.flush();
}

Expand Down Expand Up @@ -1391,3 +1461,88 @@ class VirtualScrollWithAppendOnly {
.fill(0)
.map((_, i) => i);
}

@Component({
template: `
<div cdkVirtualScrollingElement class="scrolling-element">
<cdk-virtual-scroll-viewport itemSize="50">
<div class="item" *cdkVirtualFor="let item of items">{{item}}</div>
</cdk-virtual-scroll-viewport>
</div>
`,
styles: [
`
.cdk-virtual-scroll-content-wrapper {
display: flex;
flex-direction: column;
}

.cdk-virtual-scroll-viewport {
width: 200px;
height: 200px;
background-color: #f5f5f5;
}

.item {
width: 100%;
height: 50px;
box-sizing: border-box;
border: 1px dashed #ccc;
}

.scrolling-element {
padding-top: 50px;
}
`,
],
encapsulation: ViewEncapsulation.None,
})
class VirtualScrollWithCustomScrollingElement {
@ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;
itemSize = 50;
items = Array(20000)
.fill(0)
.map((_, i) => i);
}

@Component({
template: `
<div class="before-virtual-viewport"></div>
<cdk-virtual-scroll-viewport scrollWindow itemSize="50">
<div class="item" *cdkVirtualFor="let item of items">{{item}}</div>
</cdk-virtual-scroll-viewport>
`,
styles: [
`
.cdk-virtual-scroll-content-wrapper {
display: flex;
flex-direction: column;
}

.cdk-virtual-scroll-viewport {
width: 200px;
height: 200px;
background-color: #f5f5f5;
}

.item {
width: 100%;
height: 50px;
box-sizing: border-box;
border: 1px dashed #ccc;
}

.before-virtual-viewport {
height: 50px;
}
`,
],
encapsulation: ViewEncapsulation.None,
})
class VirtualScrollWithScrollableWindow {
@ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;
itemSize = 50;
items = Array(20000)
.fill(0)
.map((_, i) => i);
}
Loading