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

Feature request: API to load external styles imperatively #3123

Closed
zerodevx opened this issue Dec 2, 2015 · 18 comments
Closed

Feature request: API to load external styles imperatively #3123

zerodevx opened this issue Dec 2, 2015 · 18 comments

Comments

@zerodevx
Copy link

zerodevx commented Dec 2, 2015

Hello. I'm not sure if there's an existing API for this. The task is to selectively load external stylesheets during runtime; the use case is to allow setting a custom element's theme declaratively (among some others I can think of).

For example, I can do

<x-test theme="super-cool-theme"></x-test>

<x-test> can then selectively load the styles (among 100's of theme selections) to apply.

Currently the pattern for loading external styles uses style-modules. However, one cannot simply do a .importHref("my-styles.html"), create a <style include="my-styles"> node and push it into local DOM.

Jsbin: http://jsbin.com/gemitewumi/edit?html,output

So I was wondering is this a bug, a feature that could be added, or there's a better way to handle this?

Thanks so much.

@zoechi
Copy link

zoechi commented Dec 2, 2015

Your example works if you enable shadow DOM. Looks like a dup of #2681

@zoechi
Copy link

zoechi commented Dec 2, 2015

Forgot to mention that you need to create a <style is="custom-style">.
I used var el = document.createElement("style", "custom-style"); in your example.

@zerodevx
Copy link
Author

zerodevx commented Dec 8, 2015

@zoechi thanks for the shout-out. AFAIK, custom-style should be used only in the main doc (index.html), not at the element-level - that's to ensure Polymer shims what's wrapped inside the <style> tag when it's not inside a Polymer element; and therefore will be a good place to put global scope mixin definitions.

My jsbin doesn't work (under both shadow and shady) because the shimming is done during element registration time.

There are possible workarounds:

  1. Ajax load external themes in custom mixins format, wrap them in <style is="custom-style"> and imperatively append them into the main doc scope;
  2. or Ajax load, add them into this.customStyle and call this.updateStyles();

but both approaches feel clunky. Since include="my-style-module" is introduced, I thought it'll be nicer to make use of it even during runtime. Perhaps we can use this issue to propose this feature (and hopefully gain traction) since it's clear to the project owners exactly what we're hoping for.

Thanks so much for your input!
J

@zoechi
Copy link

zoechi commented Dec 8, 2015

custom-style should be used only in the main doc (index.html), not at the element-level

That might be, but for me using <style is="custom-style" include="some-theme"> to add styles dynamically to elements, was the only way I could make work what I needed and it works fine so far.

@zerodevx
Copy link
Author

zerodevx commented Dec 8, 2015

Hmm... not sure why either...

In any case, I hope the imperative loading of styles is a feature that more
people will find useful (instead of just both of us) so that this gets
under the Polymer devs' radar!

Thanks again.

On Tue, Dec 8, 2015 at 6:23 PM, Günter Zöchbauer notifications@github.com
wrote:

custom-style should be used only in the main doc (index.html), not at the
element-level

That might be, but for me using <style is="custom-style" include="some-theme"> to add styles dynamically to elements, was the only
way I could make work what I needed and it works fine so far.


Reply to this email directly or view it on GitHub
#3123 (comment).

@pwrinc
Copy link

pwrinc commented Feb 3, 2016

Me too would like to have such a feature in future releases...

@jtrs
Copy link

jtrs commented Feb 4, 2016

This feature would be useful for us too,

@hrbu
Copy link

hrbu commented Feb 5, 2016

+1

@kevinSuttle
Copy link

@zerodevx There is a related snippet in the Polymer styling docs, though it is specific to "local" DOM.

<script>
    Polymer({
      is: 'x-custom',
      changeTheme: function() {
        this.customStyle['--my-toolbar-color'] = 'blue';
        this.updateStyles();
      }
    });
  </script>

@zoechi
Copy link

zoechi commented Jun 21, 2016

@kevinSuttle that isn't loading external styles.

@cunhaalx
Copy link

cunhaalx commented Jul 1, 2016

+1

1 similar comment
@jeffkevin
Copy link

+1

@tjsavage tjsavage added the 1.x label Sep 8, 2016
@ebidel ebidel added 2.x and removed 1.x labels Sep 9, 2016
@ebidel
Copy link
Contributor

ebidel commented Sep 9, 2016

Moving to 2.0 since shadow dom v1 supports stylesheets. We should support this.

@newlukai
Copy link

newlukai commented Mar 6, 2017

+1

@sorvell sorvell added the p2 label Apr 25, 2017
@kevinpschaaf
Copy link
Member

This is really a shadycss issue. Currently the shadycss polyfill only supports scoping styles prototypically for an element (e.g., for all instances of a given element type). This is a performance optimization because the ability to scope individual instances as you would need to do to support imperatively appending stylesheets to shadowRoots is significantly a.) more complex and b.) would be very hard to make performant.

This shadycss issue covers the gap in the polyfill, and we'd consider this issue blocked on resolution of that. Please see the discussion here: webcomponents/shadycss#19

@kevinSuttle
Copy link

kevinSuttle commented Apr 26, 2017

I hate to say it, but could this be made to be performant if there was virtual CSSOM diffing in memory?

@stale
Copy link

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 13, 2020
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically closed after being marked stale. If you're still facing this problem with the above solution, please comment and we'll reopen!

@stale stale bot closed this as completed Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests