Repository navigation
Conversation
marttop
changed the title
marttop
changed the title
Deleted forgotten commented console logs.
adumesny
left a comment
adumesny
left a comment
There was a problem hiding this comment.
thanks for doing this, but this is wrong approach to check for existing styles. we know what the prev max was and can append only new styles instead.
And yes we should never delete the file UNLESS there are some styles which are column based or something that can change like margin. I haven't looked carefully on what is inserted in there...
| private _initStyles(): GridStack { | ||
| if (this._stylesId) { | ||
| Utils.removeStylesheet(this._stylesId); | ||
| if (!this._stylesId) { |
There was a problem hiding this comment.
I would move that create code to first line of _updateStyles() and change constructor to call it instead. Remove _initStyles()
| if (!el || !el.parentNode) return; | ||
| el.parentNode.removeChild(el); | ||
| } | ||
| static getAllSelectors(sheet: CSSStyleSheet) { |
There was a problem hiding this comment.
return type, method description
| var ret = []; | ||
| var rules = sheet.cssRules; | ||
| // console.log(JSON.stringify(rules)); | ||
| for(var i = 0; i < rules.length; i++) { |
There was a problem hiding this comment.
for () spacing please, same with if ()
| return ret; | ||
| } | ||
|
|
||
| static hasCSSRule(sheet: CSSStyleSheet, selector: string) { |
| sheet.addRule(selector, rules); | ||
| } else if (typeof sheet.insertRule === 'function') { | ||
| sheet.insertRule(`${selector}{${rules}}`); | ||
| if(!Utils.hasCSSRule(sheet, selector)){ |
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
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.
|
Also please combine all those into 1 commit please - you can |
|
and looks like changing the |
2 participants
Description
fixed animation issue by not recreating the entire stylesheet on every event but only when it's needed.
Checklist
yarn test)