PDA

View Full Version : [CLOSED] 3.2RC1: vbox layout inside cardlayout issue



Max_nl
24 Mar 2010, 6:33 AM
If you have a tabpanel (or other panel which uses cardlayout) that does not have activeItem set, and inside the tabpanel there is a card with vbox layout, the contents of the card is not shown upon selection of the tab.

Testcase:



<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title id='title'>Title</title>
<link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" />
<script type="text/javascript" src="../../adapter/ext/ext-base.js"></script>
<script type="text/javascript" src="../../ext-all-debug.js"></script>

<script type="text/javascript">
Ext.BLANK_IMAGE_URL = '../../resources/images/default/s.gif';

Ext.ns('Max');
Max.TestPanel = Ext.extend(Ext.TabPanel, {

initComponent: function()
{
var cfg = {
deferredRender: false,
// activeItem: 0,
items:
[{
layout: 'vbox',
title: 'tab1',
items: [{
xtype: 'panel',
html: 'bla<br>bla'
},{
xtype: 'panel',
html: 'even more bla<br>bla'
}]
}]
};

Ext.apply(this, Ext.apply(this.initialConfig, cfg));
Max.TestPanel.superclass.initComponent.apply(this, arguments);
}
});

Ext.onReady(function(){
var w = new Ext.Window({
layout: 'fit',
items: [new Max.TestPanel()],
width: 600,
height: 400
});
w.show();
});
</script>
</head>
<body>
</body>
</html>


To reproduce:

- Click on "tab1"

Expected behavior:

- Contents of tab1 is shown

Actual behavior:

- Empty tab

If you click on "tab1" and then resize the Ext.Window, the contents of the card does get displayed.

meroy
24 Mar 2010, 7:11 AM
I added layoutOnTabChange and it's working.



var cfg = {
deferredRender: false,
// activeItem: 0,
layoutOnTabChange: true,
items:
[{
layout: 'vbox',
title: 'tab1',
items: [{
xtype: 'panel',
html: 'bla<br>bla'
},{
xtype: 'panel',
html: 'even more bla<br>bla'
}]
}]
};


In 3.1.2, setActiveItem in CardLayout.js looks like this:



setActiveItem : function(item){
var ai = this.activeItem,
ct = this.container;
item = ct.getComponent(item);

// Is this a valid, different card?
if(item && ai != item){

// Changing cards, hide the current one
if(ai){
ai.hide();
if (ai.hidden !== true) {
return false;
}
ai.fireEvent('deactivate', ai);
}
// Change activeItem reference
this.activeItem = item;

// The container is about to get a recursive layout, remove any deferLayout reference
// because it will trigger a redundant layout.
delete item.deferLayout;

// Show the new component
item.show();

this.layout();

if(item.doLayout){
item.doLayout();
}
item.fireEvent('activate', item);
}
},


In 3.2, their is a slight optimization, which was also there prior to 3.1.1. Somebody had opened a bug thread for this slight optimization to be restored in CardLayout.js.



setActiveItem : function(item){
var ai = this.activeItem,
ct = this.container;
item = ct.getComponent(item);

// Is this a valid, different card?
if(item && ai != item){

// Changing cards, hide the current one
if(ai){
ai.hide();
if (ai.hidden !== true) {
return false;
}
ai.fireEvent('deactivate', ai);
}

var layout = item.doLayout && (this.layoutOnCardChange || !item.rendered);

// Change activeItem reference
this.activeItem = item;

// The container is about to get a recursive layout, remove any deferLayout reference
// because it will trigger a redundant layout.
delete item.deferLayout;

// Show the new component
item.show();

this.layout();

if(layout){
item.doLayout();
}
item.fireEvent('activate', item);
}
},

meroy
24 Mar 2010, 7:35 AM
For the dev team:

The example from the first post works with 3.1.0 without having to add layoutOnTabChange.

Jamie Avins
24 Mar 2010, 7:51 AM
The layout methodology in 3.1.0 was flawed and caused serious performance issues. If you are going to render to a hidden panel (turn off deferred rendering), you are going to have to layoutOnTabChange or manually do a layout on that panel.

Max_nl
24 Mar 2010, 8:14 AM
The layout methodology in 3.1.0 was flawed and caused serious performance issues. If you are going to render to a hidden panel (turn off deferred rendering), you are going to have to layoutOnTabChange or manually do a layout on that panel.

Well, disabling deferred rendering is a necessity, so that the cards can be rendered in the background, on application start, before the user actually needs them.
Otherwise the performance of our web application is simply not acceptable under Firefox.


Anyway, if I need to add layoutOnTabChange that is fine with me.
However I still think you need to document changes like this -that can break existing applications- in the release notes.
I had the idea that it was the vbox, instead of the cardlayout, because you only mentioned changes to that in the notes.

meroy
24 Mar 2010, 8:20 AM
For readers:

This can be a breaking change for folks out there when migrating to 3.2. The 3.1.1 and 3.1.2 does an explicit layout on tab/card activation.

A bug thread was created to fix this and was done for 3.2.

If you have an app and it requires the resizing of the window for the layout to occur, the following tips may be helpful. This is for card layout or tab panels.

1. If your app is working with 3.2, you're all set.

2. If a card layout or tab panel is not rendering on activation, try calling doLayout manually as Jamie suggested.

3. Or, for a CardLayout, setting layoutOnCardChange is done like follow:



...
layout: 'card',
layoutConfig: {
layoutOnCardChange: true,
deferredRender: true // Optional, always remember this though
},
...


4. Or, for a TabPanel:



...
xtype: 'tabpanel',
layoutOnTabChange: true,
deferredRender: true, // Optional, always remember this though
...



Try step 2 first.

[Edit]: Added deferredRender. This is helpful for large apps.

FYI: The card layout requires the 2 config options to be done inside a layoutConfig.

Jamie Avins
24 Mar 2010, 8:23 AM
Good point, we'll be sure to highlight in the release notes for 3.2.

meroy
24 Mar 2010, 8:41 AM
Otherwise the performance of our web application is simply not acceptable under Firefox.


Have you tried adding a DOCTYPE line to your app. I and other folks have reported up to 40% increase in performance when having FF run in compliant mode.



<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">


Or



<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">


I do this as I have users on IE 6 and I want that to run in quirks mode, but I want Firefox to kick butt. Therefore, I need to keep the DOCTYPE line.



<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
...


The above will cause IE 6 to remain in quirks mode.

Max_nl
24 Mar 2010, 9:11 AM
Have you tried adding a DOCTYPE line to your app. I and other folks have reported up to 40% increase in performance when having FF run in compliant mode.


Already had a DOCTYPE, and changing it doesn't seem to help.

e.g. I'm using a two page cardlayout based wizard somewhere, that has a lot of checkboxes in it, and it takes 4.5 seconds to render on my Atom 330 test box.



Transitional:

Profile (4586.792ms, 32575 calls)
DomHelper() 890 17.35% 795.888ms 805.798ms 0.905ms 0.066ms 61.28ms
ext-all.js (line 8)
DomHelper() 83 15.72% 720.938ms 746.427ms 8.993ms 0.048ms 155.011ms
ext-all.js (line 8)
apply() 1 12.13% 556.57ms 556.781ms 556.781ms 556.781ms 556.781ms
ext-base.js (line 8)

Strict:

Profile (4583.34ms, 32188 calls)
DomHelper() 890 17.89% 819.841ms 829.741ms 0.932ms 0.065ms 68.18ms
ext-all.js (line 8)
DomHelper() 83 15.48% 709.714ms 735.913ms 8.866ms 0.047ms 153.339ms
ext-all.js (line 8)
apply() 1 12.01% 550.488ms 550.703ms 550.703ms 550.703ms 550.703ms
ext-base.js (line 8)


While it shows up instantly in Chrome.

meroy
24 Mar 2010, 10:28 AM
Here's one override to help increase the initial rendering time. It's actually a single line change.



Ext.override(Ext.Container, {
afterRender: function(){
// Render this Container, this should be done before setLayout is called which
// will hook onResize
Ext.Container.superclass.afterRender.call(this);
if(!this.layout){
this.layout = 'auto';
}
if(Ext.isObject(this.layout) && !this.layout.layout){
this.layoutConfig = this.layout;
this.layout = this.layoutConfig.type;
}
if(Ext.isString(this.layout)){
this.layout = new Ext.Container.LAYOUTS[this.layout.toLowerCase()](this.layoutConfig);
}
this.setLayout(this.layout);

// If a CardLayout, the active item set
if(this.activeItem !== undefined){
var item = this.activeItem;
delete this.activeItem;
this.layout.setActiveItem(item);
}

// If we have no ownerCt, render and size all children
if(!this.ownerCt){
this.doLayout(false, this.forceLayout);
}

// This is a manually configured flag set by users in conjunction with renderTo.
// Not to be confused with the flag by the same name used in Layouts.
if(this.monitorResize === true){
Ext.EventManager.onWindowResize(this.doLayout, this, [false]);
}
}
});

Max_nl
24 Mar 2010, 10:37 AM
Here's one override to help increase the initial rendering time. It's actually a single line change.


Thanks.
Saves only 0.3 sec, but all small bits help.


Profile (4237.245ms, 28840 calls)

meroy
24 Mar 2010, 10:53 AM
In that case, try this. Place all of this into an overrides.js file and include that after ext-all.js or ext-all-debug.js.



Ext.override(Ext.Container, {

afterRender: function(){
// Render this Container, this should be done before setLayout is called which
// will hook onResize
Ext.Container.superclass.afterRender.call(this);
if(!this.layout){
this.layout = 'auto';
}
if(Ext.isObject(this.layout) && !this.layout.layout){
this.layoutConfig = this.layout;
this.layout = this.layoutConfig.type;
}
if(Ext.isString(this.layout)){
this.layout = new Ext.Container.LAYOUTS[this.layout.toLowerCase()](this.layoutConfig);
}
this.setLayout(this.layout);

// If a CardLayout, the active item set
if(this.activeItem !== undefined){
var item = this.activeItem;
delete this.activeItem;
this.layout.setActiveItem(item);
}

// If we have no ownerCt, render and size all children
if(!this.ownerCt){
this.doLayout(false, this.forceLayout);
}

// This is a manually configured flag set by users in conjunction with renderTo.
// Not to be confused with the flag by the same name used in Layouts.
if(this.monitorResize === true){
Ext.EventManager.onWindowResize(this.doLayout, this, [false]);
}
}

});

Ext.layout.AutoLayout = Ext.extend(Ext.layout.ContainerLayout, {
type: 'auto',

runLayout: function(){
var ct = this.container;
ct.doLayout();
delete ct.layoutPending;
}

});

Ext.Container.LAYOUTS['auto'] = Ext.layout.AutoLayout;

Jamie Avins
24 Mar 2010, 10:54 AM
Firefox is very susceptible to redundant layouts (internal ones due to getComputedStyle) and the way we currently implementing border box via javascript is preventing optimizations. In 3.3 I'm looking to do the border box via css which should drop the unnecessary internal layout invalidation which is happening now. Webkit will only get some of this benefit since it currently has a nasty bug with margin right calculations, but it's layout engine is so much faster anyway it's not a big issue there.

stever
24 Mar 2010, 1:00 PM
For readers:

This can be a breaking change for folks out there when migrating to 3.2. The 3.1.1 and 3.1.2 does an explicit layout on tab/card activation.


Migration from 3.0 to 3.2 should not be a problem though, since it is only 3.1 that has the bug.

stever
24 Mar 2010, 7:12 PM
Firefox is very susceptible to redundant layouts (internal ones due to getComputedStyle) and the way we currently implementing border box via javascript is preventing optimizations. In 3.3 I'm looking to do the border box via css which should drop the unnecessary internal layout invalidation which is happening now. Webkit will only get some of this benefit since it currently has a nasty bug with margin right calculations, but it's layout engine is so much faster anyway it's not a big issue there.

;)

And IE9, depending on if you use currentStyle or getComputedStyle.

meroy
25 Mar 2010, 8:29 AM
I removed doLayout and onResize from the override posted in post #12. The suspendLayoutResize logic was too aggressive.

Max_nl
29 Mar 2010, 6:46 AM
One more issue guys.
If I add layoutOnTabChange like suggested, it works fine in Firefox and Chrome, but still breaks when using IE8.



Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.0; WOW64; Trident/4.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729)
Timestamp: Mon, 29 Mar 2010 14:40:01 UTC


Message: Invalid argument.
Line: 7002
Char: 9
Code: 0
URI: file:///H:/ext-3.2-rc/ext-all-debug.js

Code used:



<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title id='title'>Title</title>
<link rel="stylesheet" type="text/css" href="../../resources/css/ext-all.css" />
<script type="text/javascript" src="../../adapter/ext/ext-base.js"></script>
<script type="text/javascript" src="../../ext-all-debug.js"></script>

<script type="text/javascript">
Ext.BLANK_IMAGE_URL = '../../resources/images/default/s.gif';

Ext.ns('Max');
Max.TestPanel = Ext.extend(Ext.TabPanel, {

initComponent: function()
{
var cfg = {
deferredRender: false,
layoutOnTabChange: true,

items:
[{
layout: 'vbox',
title: 'tab1',
items: [{
xtype: 'panel',
html: 'bla<br>bla'
},{
xtype: 'panel',
html: 'even more bla<br>bla'
}]
}]
};

Ext.apply(this, Ext.apply(this.initialConfig, cfg));
Max.TestPanel.superclass.initComponent.apply(this, arguments);
}
});

Ext.onReady(function(){
var w = new Ext.Window({
layout: 'fit',
items: [new Max.TestPanel()],
width: 600,
height: 400
});
w.show();
});
</script>
</head>
<body>
</body>
</html>

Jamie Avins
29 Mar 2010, 8:01 AM
[QUOTE=Max_nl;452225]One more issue guys.
If I add layoutOnTabChange like suggested, it works fine in Firefox and Chrome, but still breaks when using IE8.



Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.0; WOW64; Trident/4.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729)
Timestamp: Mon, 29 Mar 2010 14:40:01 UTC


Message: Invalid argument.
Line: 7002
Char: 9
Code: 0
URI: file:///H:/ext-3.2-rc/ext-all-debug.js


This issue has been resolved in svn already (was present in the rc1 release).

PTG
3 Apr 2010, 3:37 AM
Thank you meroy, your override on post #12 fixed a lot of problem for me.

Since I updated to 3.2, the layouts were extremely slow (almost 2sec to show a window containing a form panel and nested tabs)
Also I didn't want to activate a tab by default and since 3.2, I could see the form fields on top of my tab panel even with no tab was activated, leading to more problems when loading the form data.
I was about to revert back to 3.1 but your override fixes all those problems.

meroy
4 Apr 2010, 12:06 AM
Thank you meroy, your override on post #12 fixed a lot of problem for me.


Glad to hear that the override is working for you.
The 3.2 release should have worked as well though.

The problem may be coming from the changes made to 3.2's AutoLayout.js which the override reverts to a prior release.

PTG
4 Apr 2010, 3:20 PM
The 3.2 release should have worked as well though.

Do you suggest that I'm doing something wrong ?
Any tips on what kind of mistake I should check ?

meroy
4 Apr 2010, 4:10 PM
Do you suggest that I'm doing something wrong ?
Any tips on what kind of mistake I should check ?

It's hard to tell without a test case. Posting a test case will give you a piece of mind in the end.

It may be coming from 3.2 and therefore -- it's a bug -- the test case will be helpful to Jamie for edge testing. Basically, your app should work at some point without the need for an override.