[FIXED][3.0RC1]bodyStyle not applied padding

[6epcepk]

Hi, with ext 2.2.1 working great, but now padding not applied for any cmp.
For example:

Ext.Window({
bodyStyle: 'background: #cad9ec; padding: 7px 7px 0 7px',
});
New config attributes or anything else?

[Condor]

No, applyStyles no longer trims whitespace (should this be classified as a bug?).

This should work:

bodyStyle: 'background:#cad9ec;padding:7px 7px 0 7px',

[6epcepk]

Ohh, thank you!
IMO, this is bug, because clean code markup with whitespaces is good practice.

[Condor]

I'll move this thread to the bugs section...

[evant]

Fixed in SVN, strings will now be trimmed appropriately.

[Animal]

I'm not sure that the solution is the best. Perhaps some benchmarking could be done?

Generating a trim function on every trip through applyStyles must be a bit inefficient. Wouldn't using String's Ext-supplied trim call on each element be better?

[evant]

I've modified the implementation slightly. The problem it was having is trying to match pairs when having trailing spaces. If we trim the initial input then split on :|;, then we can just trim the values and be sure they aren't null.

[Animal]

I see that each individual element of the Array must be trimmed. But given that we are using a RegExp to split on, that could be extended to "consume" any whitespace:



applyStyles : function(el, styles){
if(styles){
var i = 0,
len;

el = Ext.fly(el);
if(Ext.isFunction(styles)){
styles = styles.call();
}
if (typeof styles == "string") {
styles = styles.split(/\s*(?::|;)\s*/g);
for (len = styles.length; i < len;) {
el.setStyle(styles[i++], styles[i++]);
}
}else if (Ext.isObject(styles)){
el.setStyle(styles);
}
}
},

[Animal]

Is it important that a style property is set to null for an empty style specification?

If so, then



applyStyles : function(el, styles){
if(styles){
var i = 0,
len;

el = Ext.fly(el);
if(Ext.isFunction(styles)){
styles = styles.call();
}
if (typeof styles == "string") {
styles = styles.split(/\s*(?::|;)\s*/g);
for (len = styles.length; i < len;) {
el.setStyle(styles[i++]||null, styles[i++]||null);
}
}else if (Ext.isObject(styles)){
el.setStyle(styles);
}
}
},

[aconran]

It's worthwhile to take a look at any possible optimizations here but we should be trimming out the whitespace for all of these styles.

[Animal]

That's what this line does:



styles = styles.split(/\s*(?::|;)\s*/g);


Without the need for a subsequent call to trim()

[mystix]

That's what this line does:



styles = styles.split(/\s*(?::|;)\s*/g);


Without the need for a subsequent call to trim()
+1

@animal's regex correctly trims strings on split.
could be shorter though (no need for the global flag since String.prototype.split() already calls the regex on the entire string):


styles = styles.split(/\s*[:;]\s*/);

[evant]

I don't think that works as intended:



var styles = ' padding: 10px; ';
styles = styles.split(/\s*(?::|;)\s*/g);
console.log(styles);


You end up with



[' padding', '10px', '']


Perhaps this will work better:



var styles = ' padding : 10px; border-left : 10px; ';
styles = styles.trim().split(/\s*(?::|;)\s+/g);
console.log(styles);


No need for a trim call on each item.

[mystix]

I don't think that works as intended:



var styles = ' padding: 10px; ';
styles = styles.split(/\s*(?::|;)\s*/g);
console.log(styles);


You end up with



[' padding', '10px', '']


Perhaps this will work better:



var styles = ' padding : 10px; border-left : 10px; ';
styles = styles.trim().split(/\s*(?::|;)\s+/g);
console.log(styles);


No need for a trim call on each item.
yep, that initial call to trim() is required.
note: the global flag can be omitted. see my post above.

[evant]

Added this into svn.

[Animal]

That won't work

The "+" means that it requires at least one whitespace after the (colon or semicolon)

http://i131.photobucket.com/albums/p286/TimeTrialAnimal/temp.jpg

RegExps are one of the things (like XTemplates) that you can fine tune and get just right by farting around with them on the Firebug console command line.

[evant]

Yeah, typo, I was screwing around with a few different things and forgot to revert that. Cheers.

[mschwartz]

Just an FYI

This one bit me pretty badly. I generally develop in Firebug and test in IE7 periodically. It's been maybe since RC1 came out since I tested in IE.

Load it up in IE, get an error:
"could not get the textAlign property. Invalid argument"

Debugging in IE, I find it's trying to set textAlign to " left"

IE doesn't like it.

Added this to my Ext.overrides.js and it's fixed:



// required for Ext-3.0-rc1.1
// fix for http://extjs.com/forum/showthread.php?t=66120
Ext.apply(Ext.DomHelper, {
applyStyles : function(el, styles){
if(styles){
var i = 0,
len;

el = Ext.fly(el);
if(Ext.isFunction(styles)){
styles = styles.call();
}
if (typeof styles == "string") {
styles = styles.split(/\s*(?::|;)\s*/g);
for (len = styles.length; i < len;) {
el.setStyle(styles[i++]||null, styles[i++]||null);
}
}else if (Ext.isObject(styles)){
el.setStyle(styles);
}
}
}
});

[evant]

A very similar thing exists in svn.