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

refactor(material/sidenav): undeprecate constructor signature #20589

Merged
merged 1 commit into from Sep 23, 2020

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Sep 16, 2020

Removes the deprecation labels from a constructor parameter since it's tricky to
sync into g3 and is easy to maintain on our end.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: major This PR is targeted for the next major release labels Sep 16, 2020
@crisbeto crisbeto added this to the 11.0.0 milestone Sep 16, 2020
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 16, 2020
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Sep 16, 2020
@wagnermaciel
Copy link
Contributor

@crisbeto Please resolve this merge conflict

@crisbeto
Copy link
Member Author

Rebased.

@annieyw
Copy link
Contributor

annieyw commented Sep 21, 2020

Apps using <mat-sidenav> without a container would have trouble making this change

@crisbeto
Copy link
Member Author

I was under the impression that we require sidenavs to be inside a container. If that's not the case we can un-deprecate the signature. Can you weigh in @mmalerba?

@jelbourn
Copy link
Member

@crisbeto this came up because someone is actually doing it- we found an instance where mat-sidenav is the root element in a component. We'll have to fix them before we can move this ahead

@crisbeto
Copy link
Member Author

I was asking, because I had the impression that we needed a sidenav inside a container, but if that's not the case and we want to support it, keeping the parameter optional won't be a big deal.

@jelbourn
Copy link
Member

I don't feel strongly either way; it certainly was never intended to work, but it's probably not worth the effort to fix it

@crisbeto
Copy link
Member Author

crisbeto commented Sep 23, 2020

Okay, in that case I'll revert the changes and turn this PR into a merge-safe one that un-deprecates the old signature.

@crisbeto crisbeto force-pushed the sidenav-v11-breaking-changes branch 2 times, most recently from 0e9a0e0 to 5ab0bd1 Compare September 23, 2020 09:01
@crisbeto crisbeto changed the title refactor(sidenav): remove deprecated APIs for v11 refactor(material/sidenav): undeprecate constructor signature Sep 23, 2020
@crisbeto crisbeto added merge safe P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release and removed P2 The issue is important to a large percentage of users, with a workaround target: major This PR is targeted for the next major release labels Sep 23, 2020
@crisbeto crisbeto removed this from the 11.0.0 milestone Sep 23, 2020
Removes the deprecation labels from a constructor parameter since it's tricky to
sync into g3 and is easy to maintain on our end.
@annieyw annieyw merged commit 993767f into angular:master Sep 23, 2020
annieyw pushed a commit that referenced this pull request Sep 23, 2020
Removes the deprecation labels from a constructor parameter since it's tricky to
sync into g3 and is easy to maintain on our end.

(cherry picked from commit 993767f)
wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Sep 24, 2020
…r#20589)

Removes the deprecation labels from a constructor parameter since it's tricky to
sync into g3 and is easy to maintain on our end.
@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 Oct 24, 2020
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 P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants