ducksboard / gridster.js

gridster.js is a jQuery plugin that makes building intuitive draggable layouts from elements spanning multiple columns
http://gridster.net/
MIT License
6.03k stars 1.2k forks source link

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

Closed mattacular closed 10 years ago

mattacular commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 10 years ago

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

techoshi commented 10 years ago

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 commented 10 years ago

Thanks this was helpful - solved the doubling up issue

Superdrac commented 10 years ago

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

Anyway, thank you !!