PDA

View Full Version : [FIXED][3.0RC1]bodyStyle not applied padding



6epcepk
21 Apr 2009, 12:25 AM
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
21 Apr 2009, 12:36 AM
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
21 Apr 2009, 12:40 AM
Ohh, thank you!
IMO, this is bug, because clean code markup with whitespaces is good practice.

Condor
21 Apr 2009, 12:59 AM
I'll move this thread to the bugs section...

evant
30 Apr 2009, 3:45 PM
Fixed in SVN, strings will now be trimmed appropriately.

Animal
30 Apr 2009, 11:34 PM
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
30 Apr 2009, 11:46 PM
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
1 May 2009, 12:07 AM
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
1 May 2009, 12:13 AM
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
5 May 2009, 11:59 AM
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
5 May 2009, 12:25 PM
That's what this line does:



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


Without the need for a subsequent call to trim()

mystix
5 May 2009, 7:14 PM
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
5 May 2009, 7:26 PM
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
5 May 2009, 7:53 PM
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
5 May 2009, 9:46 PM
Added this into svn.

Animal
6 May 2009, 12:56 AM
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
6 May 2009, 1:24 AM
Yeah, typo, I was screwing around with a few different things and forgot to revert that. Cheers.

mschwartz
15 May 2009, 8:37 AM
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
15 May 2009, 4:34 PM
A very similar thing exists in svn.