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.

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

fix(sidenav): scrollable instance not exposed when explicitly specifying content element #11517

Merged

Conversation

crisbeto
Copy link
Member

  • 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.

…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 requested a review from mmalerba as a code owner May 25, 2018 19:27
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 25, 2018
@mmalerba mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels May 29, 2018
@andrewseguin andrewseguin merged commit 436ac7d into angular:master Jun 7, 2018
andrewseguin added a commit that referenced this pull request Jun 7, 2018
… specifying content element (#11517)"

This reverts commit 436ac7d.
andrewseguin added a commit that referenced this pull request Jun 7, 2018
… specifying content element (#11517)" (#11697)

This reverts commit 436ac7d.
@andrewseguin
Copy link
Contributor

D'oh - sorry @crisbeto, I merged this before realizing it was a target: major. I reverted the merge with #11697. Sorry for the inconvenience but can you re-open this PR?

mmalerba pushed a commit that referenced this pull request Jul 29, 2018
…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.
@swftvsn
Copy link
Contributor

swftvsn commented Oct 4, 2018

@andrewseguin @crisbeto @mmalerba Could you please merge and release this as part of 7? The next window of opportunity is 8 after that, and I fear it will be quite long time to that.

Thank you for the consideration!

@mmalerba
Copy link
Contributor

mmalerba commented Oct 4, 2018

@crisbeto want to just do the @Optional ...? thing so we can get this in sometime before 8? It's already too late for 7, we're in RC

@crisbeto
Copy link
Member Author

crisbeto commented Oct 5, 2018

@mmalerba @swftvsn these changes were already merged in time for 7.0 via #11706.

@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.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MatSidenavContainer scroll events not working as documented
5 participants