Skip to content

Conversation

@LironHazan
Copy link

No description provided.

@fetis fetis added the snippet New snippet or snippet update label Jun 16, 2019
@NothingEverHappens
Copy link
Contributor

Hey Liron, thanks for the contribution!
It would be great to mention that schemas: [CUSTOM_ELEMENTS_SCHEMA] has to be included in the module.
Also would be great to have a warning saying that the type safety would be lost in terms of component names.

@LironHazan
Copy link
Author

@NothingEverHappens great, I'll mention the schemas required addition, but regarding the warning, I'm not sure I understand your comment, what do you mean by type safety? is it concerns to a typescript compiler issue recognising the custom element class type? or a name conflict in case there's an angular component with the same name of the custom element? sorry I tried to look out for more info but couldn't find something relevant so I'll appreciate if you'll give a short explanation and I'll make sure to add the warning :)

@kirjs
Copy link
Contributor

kirjs commented Jul 7, 2019

Hey, Liron, I think it you have a <element-does-not-exist> used somewhere in the template, angular would display an error saying it can't find it.

However with CUSTOM_ELEMENTS_SCHEMA, my understanding is that no error would be shown? (unless I'm missing something?)

@LironHazan
Copy link
Author

@kirjs yeah, adding CUSTOM_ELEMENTS_SCHEMA allows an NgModule to contain the following:
Non-Angular elements named with dash case (-).
Element properties named with dash case (-). Dash case is the naming convention for custom elements.
unless we'll get template parse errors, I'll add it to the PR

@LironHazan
Copy link
Author

@fetis mind review changes? :)

@kirjs kirjs changed the title feat(snippet)web component in use feat(snippet): web component in use Jul 15, 2019
this.setAttribute('text', value);
}

get color() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't seem like color is used here, can we drop it to simplify the demo?

Copy link
Author

Choose a reason for hiding this comment

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

@kirjs sorry for the delay, sure I dropped it!

@kirjs
Copy link
Contributor

kirjs commented Jul 15, 2019

@LironHazan This is good to merge, the demo seems to be working (you can check it out here: https://codelab.fun/30/new/30-seconds-of-angular/nycJSorg/189)

The only thing left - it would be awesome to simpilfy the demo a bit (throw away irrelevant stuff)

LironHazan and others added 2 commits September 9, 2019 12:05
ok thanks

Co-Authored-By: Sergey Fetiskin <fetis26@gmail.com>
uppercase

Co-Authored-By: Sergey Fetiskin <fetis26@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

snippet New snippet or snippet update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants