Skip to content

fixed animation issue by not recreating the entire stylesheet #1391

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

Closed
marttop wants to merge 2 commits into from
Closed

fixed animation issue by not recreating the entire stylesheet #1391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20f2d7e
fixed animation issue by not recreating the entire stylesheet on ever…
marttop Sep 30, 2020
a82e57a
Update utils.ts
marttop Oct 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion demo/locked.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ <h1>Locked demo</h1>
</div>

<script type="text/javascript">
let grid = GridStack.init({float: true});
let grid = GridStack.init({float: true, animate :true,});

grid.on('added removed change', function(e, items) {
let str = '';
Expand Down
11 changes: 5 additions & 6 deletions src/gridstack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,13 +1072,12 @@ export class GridStack {

/** @internal */
private _initStyles(): GridStack {
if (this._stylesId) {
Utils.removeStylesheet(this._stylesId);
if (!this._stylesId) {

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would move that create code to first line of _updateStyles() and change constructor to call it instead. Remove _initStyles()

this._stylesId = 'gridstack-style-' + (Math.random() * 100000).toFixed();
// insert style to parent (instead of 'head' by default) to support WebComponent
let styleLocation = this.opts.styleInHead ? undefined : this.el.parentNode as HTMLElement;
this._styles = Utils.createStylesheet(this._stylesId, styleLocation);
}
this._stylesId = 'gridstack-style-' + (Math.random() * 100000).toFixed();
// insert style to parent (instead of 'head' by default) to support WebComponent
let styleLocation = this.opts.styleInHead ? undefined : this.el.parentNode as HTMLElement;
this._styles = Utils.createStylesheet(this._stylesId, styleLocation);
if (this._styles !== null) {
this._styles._max = 0;

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should NEVER happen but on creation, then we can use the max to figure new styles that need to be appended.
you don't need to check for existing styles.

}
Expand Down
32 changes: 28 additions & 4 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,37 @@ export class Utils {
if (!el || !el.parentNode) return;
el.parentNode.removeChild(el);
}
static getAllSelectors(sheet: CSSStyleSheet) {

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

return type, method description

var ret = [];
var rules = sheet.cssRules;
for(var i = 0; i < rules.length; i++) {

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for () spacing please, same with if ()

var rule = rules[i];
var index = rule.cssText.indexOf(" {");
if(index >0){
var sel = rule.cssText.substr(0,index);
ret.push(sel);
}
}
return ret;
}

static hasCSSRule(sheet: CSSStyleSheet, selector: string) {

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

var selectors = Utils.getAllSelectors(sheet);
for(var i = 0; i < selectors.length; i++) {
if(selectors[i] == selector){
return true;
}
}
return false;
}
/** inserts a CSS rule */
static addCSSRule(sheet: CSSStyleSheet, selector: string, rules: string) {
if (typeof sheet.addRule === 'function') {
sheet.addRule(selector, rules);
} else if (typeof sheet.insertRule === 'function') {
sheet.insertRule(`${selector}{${rules}}`);
if(!Utils.hasCSSRule(sheet, selector)){

@adumesny adumesny Oct 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I actually don't know what we need to check for existing rules... I mean we know what the previous max was so we can append new styles only. Also the original one are created once so they don't need this.

if (typeof sheet.addRule === 'function') {
sheet.addRule(selector, rules);
} else if (typeof sheet.insertRule === 'function') {
sheet.insertRule(`${selector}{${rules}}`);
}
}
}

Expand Down