-
Notifications
You must be signed in to change notification settings - Fork 34
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
@IncludeStyles does not work on non-root component #89
Comments
Seems like the component factory is not available when I'm scanning for it. A quick workaround is to add the components using That fixes for JIT compilation, but neither component has the styling with AOT compilation. I'm going to have to dig into that one further. Not sure if that's an existing bug, or something with the newer versions of Angular. |
I've got a fix for AOT not working, but we come back to the problem of the component factory. We could document and require that any component using We really just need to associate an HTML selector string with one or more Polymer style modules. The only reason I'm using a decorator and component factory is so that it improves the developer experience. There's no reason we can't do something like this: IncludeStyles.for('app-child')('app-colors');
@Component({
selector: 'app-child'
})
export class ChildComponent {} It feels "side effect"-y to me, but it'd solve the problem and could get rid of a lot of code that is trying to dynamically look up the selector provided to Using a decorator "feels" much better, but I can't think of a clever way to link a class's type to it's component metadata without the component factory. If you have any suggestions, I would appreciate it! |
Thank you for your time and inputs! I digged further into the issue and it turned out it is a different issue. I use vaadin components so I'm not yet sure if the problem might be on their side... Is the solution that you have to apply some ShadyCSS functions from within your Hope that explanation is not too confusing... Note that when the global style-include for 'lumo-color' is deactivated (app.component.ts line 10), then the dom-module style is not "erased" and the @IncludeStyles works as expected) |
Good find! Looks like cssFromTemplate() does remove the style node from the DomModule element. That makes sense, since we don't want those styles leaking in legacy browsers. I could change IncludeStyles to use cssFromModules() to get the CSS string instead of manually querying it. |
That looks right to me on first sight (but I am a real novice what concerns Polymer and ShadyCSS). |
ShadyCSS doesn't touch Polymer style modules, so there's no step there I have to worry about. Origami already checks and adds stylesheets to ShadyCSS in another module. All IncludeStyles does is add the string to the component styles, which would get passed through ShadyCSS later. |
Seems to work fine :-)
where stylesFromModule is imported from Shall I issue a PR? |
Sure! PRs are always welcome. If we don't use |
AOT fix is pushed out and will be published shortly, but I'm leaving this open for the other problem |
For the style-related issue I issued a PR some time ago. Did you have the chance to look at it so that it can possibly be merged? |
Looks like my only comments on it were documentation, I can merge and tweak those manually. |
I'm not sure why this issue is still open. My PR was merged a long time ago (#90)... |
I left this open for some reason, but I can't think of it now! |
Somehow @IncludeStyles does not seem to work on a non-root component.
See my sample code at: /~https://github.com/akaegi/include-styles-demo.
(dom-module 'app-colors' defined in index.html)
The text was updated successfully, but these errors were encountered: