Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Shadowless DOM #936

Closed
wants to merge 2 commits into from
Closed

Conversation

jbdeboer
Copy link
Contributor

No description provided.

@jbdeboer jbdeboer added cla: yes and removed cla: no labels Apr 21, 2014
@@ -320,7 +327,8 @@ class Component extends Directive {
selector,
visibility,
exportExpressions,
exportExpressionAttrs})
exportExpressionAttrs,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you update _cloneWithNewMap below ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I wrote a test for this in 0c797cc

Copy link
Contributor

Choose a reason for hiding this comment

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

One minor thing is also to update class level docs "Angular components use shadow-DOM for rendering their templates"

@vicb
Copy link
Contributor

vicb commented Apr 22, 2014

@jbdeboer I like to have return types on all methods. It makes it easier to understand the code and can also help with auto-completion. What do you think ?

part of angular.core.dom_internal;

@proxy
class ShadowlessShadowRoot implements dom.ShadowRoot {
Copy link
Contributor

Choose a reason for hiding this comment

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

ShadowRoot extends DocumentFragment and doesn't add too much more- perhaps just use DocumentFragment instead?

Alternatively this may be a good case for a custom element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used in onShadowRoot, which needs to be renamed.

We'll rework this later.

jbdeboer added a commit that referenced this pull request Apr 22, 2014
jbdeboer added a commit that referenced this pull request Apr 22, 2014
jbdeboer added a commit that referenced this pull request Apr 23, 2014
@jbdeboer jbdeboer closed this in c60e936 Apr 23, 2014
jbdeboer added a commit that referenced this pull request Apr 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

4 participants