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.

Sign up for GitHub

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

Jump to bottom

Gridster spawns new <style> block everytime widget added when "autogenerate_stylesheet" = true #211

Closed
mattacular opened this issue Aug 10, 2013 · 7 comments · May be fixed by #352
Closed

Gridster spawns new <style> block everytime widget added when "autogenerate_stylesheet" = true #211

mattacular opened this issue Aug 10, 2013 · 7 comments · May be fixed by #352
Labels
bug

Comments

@mattacular
Copy link

It would probably be better to clear the previous style block when adding a new one since the new one will always contain the CSS to style the new row/column + all the old ones.

Otherwise you get a stack of potentially unlimited style blocks as you add widgets to the page.

Let me know if you'll take a PR on this issue.
@argshook
Copy link

Second. I'm working on dynamically added widgets and noticed this issue as well. I'm sticking with autogenerate_stylesheet = false and a copy-paste of previously generated style for now.

@argshook
Copy link

Using autogenerate_stylesheet = false wasn't an option for me since I am working with dynamically added widgets and cannot always know how many widgets might be added. So I changed add_style_tag method a bit. Here it is with comments of what I did:

fn.add_style_tag = function(css) {
  var d = document;
  var tag = d.createElement('style');

  tag.setAttribute('type', 'text/css');

  // I need to know whether style tag holds gridster stuff or not.
  // There might be other style tags in <head> so I need to somehow identify gridster one.
  // And I'm doing that by adding id attribute to style tag. I am probably going to hell for
  // this since style tag does not have id attribute. But, well, it works.
  tag.setAttribute('id', 'gridster');

  var el = d.querySelectorAll('style#gridster');

  // if I have what I need, I can swap it's innerHTML and avoid creating another style tag
  if(el.length > 0) {
    var currentStyle = el[0].innerHTML;
    if(css.length > currentStyle.length) {
      el[0].innerHTML = css;
    }
  }else{
    // if I don't have what I need, gridster has not yet created its style tag
    // and does that here
    d.getElementsByTagName('head')[0].appendChild(tag);
  }

  //the rest is not altered

  if (tag.styleSheet) {
    tag.styleSheet.cssText = css;
  }else{
    tag.appendChild(document.createTextNode(css));
  }

  this.$style_tags = this.$style_tags.add(tag);

  return this;
};

This way gridster doesn't create another style tag for every widget but instead updates the contents of it. The drawback is that it changes all of the contents while ideally it should only add the necessary new rules. But I'm not smart enough to do that yet.

It's also a bit faster. Here's a screenshot of chrome dev tools CPU profiler:

selection_202

It all works nicely, is faster and doesn't add lots of very similar style tags. But id attribute in style tag is very, very dirty.

Please don't be harsh on me as I'm not at all a professional javascripter, am very very new to git and github.
And yeah you should probably not use this in production.

I hope someone can come up with a better solution. As for now this'll work for me.

@mattacular
Copy link
Author

I implemented a similar patch. Btw "id" is a global attribute; you can use it on style and script tags (it even passes W3C validation). That's the only way you can make specific elements of those types selectable.

@mattacular
Copy link
Author

@vieron would you be open to a PR on this?

@techoshi
Copy link

Joining the conversation... I stared playing with gridster this past week. I'm using it for an application I'm building in Microsoft.net!

Happy Coding!

@bostondevin
Copy link

Thanks this was helpful - solved the doubling up issue

dasmall added a commit to dasmall/gridster.js that referenced this issue Apr 10, 2014
@dasmall
Removing previously added style tags before adding new one.
1c3659c 
Hopefully fixes ducksboard#211 and ducksboard#294.
@dasmall dasmall mentioned this issue Apr 10, 2014
Open
@Superdrac
Copy link

Worked fine for me !! Why this update is not add in the main repo ?

Anyway, thank you !!

@vieron vieron added the bug label Jun 16, 2014
@vieron vieron closed this as completed in 93c46ff Jun 16, 2014
dsmorse pushed a commit to dsmorse/gridster.js that referenced this issue Apr 8, 2015
Merge remote-tracking branch 'dasmall/gridster.js.git/master'
524ed2b 
* dasmall/gridster.js.git/master:
  Removing previously added style tags before adding new one. Hopefully fixes ducksboard#211 and ducksboard#294.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removing previously added style tags before adding new one. dasmall/gridster.js
6 participants
@vieron @bostondevin @mattacular @argshook @techoshi @Superdrac