Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MatSidenavContainer scroll events not working as documented #10884

Closed
mackelito opened this issue Apr 16, 2018 · 6 comments · Fixed by #11706 or #11517
Closed

MatSidenavContainer scroll events not working as documented #10884

mackelito opened this issue Apr 16, 2018 · 6 comments · Fixed by #11706 or #11517
Assignees
@crisbeto @mmalerba
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@mackelito
Copy link

mackelito commented Apr 16, 2018 •
edited
Loading

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Able to react to scrolling inside the <mat-sidenav-container>

What is the current behavior?

Unhandled Promise rejection: Cannot read property 'scrollable' of undefined ; Zone: ; Task: Promise.then ; Value: Error: Cannot read property 'scrollable' of undefined TypeError: Cannot read property 'scrollable' of undefined at new SidenavResponsiveExample

What are the steps to reproduce?

https://stackblitz.com/edit/angular-rgkuk8?file=app/sidenav-responsive-example.ts

What is the use-case or motivation for changing an existing behavior?

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Is there anything else we should know?
@stevo-knievo
Copy link

it's still broken in 6.1.0.

Here the example:
https://stackblitz.com/edit/sidenavcontainer-scrollable-elementscrolled

@mmalerba
Copy link
Contributor

It looks like the query listener is being added too soon in the example, it should be deferred until ngOnInit: https://stackblitz.com/edit/angular-rgkuk8-ssckuv?file=app/sidenav-responsive-example.ts

Would anyone like to send a PR updating the example?

@mmalerba mmalerba added help wanted The team would appreciate a PR from the community to address this issue docs This issue is related to documentation labels May 23, 2018
@stevo-knievo
Copy link

Sorry I don't understand. My example has no query listener. The problem is that "Reacting to scroll events inside the sidenav container" as described here doesn't work respectively it throws an error: Error: this.sidenavContainer is undefined

@mmalerba mmalerba added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed docs This issue is related to documentation help wanted The team would appreciate a PR from the community to address this issue labels May 24, 2018
@mmalerba
Copy link
Contributor

@stevo-knievo ok sorry I think I misunderstood. So there's a couple things. First, in your demo the sidenav content is not actually the element that's scrolling. The sidenav container is expanding to fill its content and the body is scrolling. You would need to size it so the content element is the element that's scrolling.

The second issue appears to be a problem with Angular Material. It looks like we only grab the CdkScrollable when the <mat-sidenav-content> is not explicitly passed. A workaround for this is to just add the cdkScrollable directive to the <mat-sidenav-content> that you pass in and then grab that with a @ViewChild(). We should fix this though, so leaving this issue open to track it.

Here's a version of your example where I've absolutely positioned the sidenav container to restrict its height and added the cdkScrollable to the content element: https://stackblitz.com/edit/sidenavcontainer-scrollable-elementscrolled-jqg8ob?file=src/app/app.component.html

crisbeto added a commit to crisbeto/material2 that referenced this issue May 25, 2018
@crisbeto
fix(sidenav): scrollable instance not exposed when explicitly specify…
150f565 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes angular#10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
@crisbeto crisbeto mentioned this issue May 25, 2018
Merged
josephperrott pushed a commit that referenced this issue May 30, 2018
@crisbeto @josephperrott
fix(sidenav): scrollable instance not exposed when explicitly specify…
2d4bc0b 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes #10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
andrewseguin pushed a commit that referenced this issue Jun 5, 2018
@crisbeto @andrewseguin
fix(sidenav): scrollable instance not exposed when explicitly specify…
ee61ab5 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes #10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
andrewseguin pushed a commit that referenced this issue Jun 7, 2018
@crisbeto @andrewseguin
fix(sidenav): scrollable instance not exposed when explicitly specify…
4852f50 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes #10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
andrewseguin pushed a commit that referenced this issue Jun 7, 2018
@crisbeto @andrewseguin
fix(sidenav): scrollable instance not exposed when explicitly specify…
436ac7d 
…ing content element (#11517)

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes #10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 7, 2018
@crisbeto
fix(sidenav): scrollable instance not exposed when explicitly specify…
3509cf0 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes angular#10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jun 7, 2018
@crisbeto
fix(sidenav): scrollable instance not exposed when explicitly specify…
a5eef4a 
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes angular#10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
@crisbeto crisbeto mentioned this issue Jun 7, 2018
Merged
@crisbeto
Copy link
Member

crisbeto commented Jun 7, 2018

Reopening the issue since the PR got in by accident and had to be reverted.

@crisbeto crisbeto reopened this Jun 7, 2018
mmalerba pushed a commit that referenced this issue Jul 29, 2018
@crisbeto @mmalerba
fix(sidenav): scrollable instance not exposed when explicitly specify…
441c98e 
…ing content element (#11706)

This is a resubmit of #11517 which got in by accident.

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes #10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@crisbeto crisbeto

@mmalerba mmalerba

Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Milestone
No milestone
4 participants
@mackelito @stevo-knievo @crisbeto @mmalerba