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(schematics): do not allow specifying native view encapsulation #12632

Merged

Conversation

devversion
Copy link
Member

@devversion devversion commented Aug 11, 2018

No longer allows developers to specify the --viewEncapsulation Native when creating components through the Angular Material schematics. Since Angular Material does not work properly with Native / ShadowDom view encapsulation, we should not support these as valid schematic options.

@devversion devversion added pr: merge safe target: major This PR is targeted for the next major release labels Aug 11, 2018
@devversion devversion requested a review from amcdnl as a code owner August 11, 2018 11:36
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 11, 2018
@jelbourn
Copy link
Member

I don't know if this is a good idea; our theming doesn't work with native shadow DOM.

@devversion
Copy link
Member Author

devversion commented Aug 13, 2018

The option is just setting the ShadowDom encapsulation on the given component that has been created. The idea was to make everything consistent with the @schematics/angular:component schematic.

I see that we should probably not support this. If we don't want to go that way, even though it's kind of bad for consistency, we should also consider removing the Native view encapsulation from Angular versions before. Also do we have some notes on the README.md about this?

No longer allows developers to specify the `--viewEncapsulation Native` when creating components through the Angular Material schematics. Since Angular Material does not work properly with Native / ShadowDom view encapsulation, we should not support these as valid schematic options.
@devversion devversion force-pushed the feat/schematics-shadowdom-strategy branch from 9f21d06 to ae05678 Compare August 17, 2018 06:14
@devversion devversion changed the title feat(schematics): support ShadowDom view encapsulation fix(schematics): do not allow specifying native view encapsulation Aug 17, 2018
@devversion
Copy link
Member Author

devversion commented Aug 17, 2018

@jelbourn Addressed what we discussed in the standup. Though we do not have a good way of throwing an error because we need to handle the possible values through the schema.

Otherwise, tools that show the available options for a schematic (e.g. AngularConsole) will incorrectly display ShadowDom or Native as valid options.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 20, 2018
@jelbourn jelbourn merged commit 0a823dd into angular:master Aug 21, 2018
@devversion devversion deleted the feat/schematics-shadowdom-strategy branch August 21, 2018 09:01
@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.

3 participants