Opened 10 years ago

Closed 10 years ago

#10241 closed defect (fixed)

[patch] [cla] [regression] GridContainer resize problem with StackContainer child

Reported by: Mathevet julien Owned by: dante
Priority: high Milestone: 1.4
Component: Dojox Version: 1.4.0b
Keywords: Cc: easyrasta@…
Blocked By: Blocking:

Description

I have a GridContainer? with 2 column. And each column take screen size... The resize method of LayoutContainer? doesn't detect gridContainer has parent.

Attachments (2)

test_gridContainer_TabContainer.html (6.0 KB) - added by Mathevet julien 10 years ago.
patchGridContainer.patch (27.5 KB) - added by Mathevet julien 10 years ago.

Download all attachments as: .zip

Change History (16)

Changed 10 years ago by Mathevet julien

comment:1 Changed 10 years ago by Mathevet julien

And in this example tabContainer isn't shown also.

comment:2 Changed 10 years ago by Adam Peller

Owner: changed from Adam Peller to dante

comment:3 Changed 10 years ago by Mathevet julien

Seem adding isLayoutContainer: true, to GridContainer? fix problem.

In fact Child widgets are loaded before, grid initialize column.

comment:4 Changed 10 years ago by Mathevet julien

This patch fix build order and child startup. It also fix: #10244

comment:5 Changed 10 years ago by Mathevet julien

This patch doesn't fix this example, but it fixs many of mines. I use a widget that extends StackContainer. And works for all of yours. I have juste reorganized build order. And this is repect more "widget layout".

comment:6 Changed 10 years ago by Mathevet julien

Main problem in your svn version is the GridContainer? doesn't respect the new design of inheritance of StackContainer?. I fix it and fix startup childs. I have also reduced size code, removing unused or duplicated variable. I hope you will add this patch for 1.4 version. Currently your version doesn't work for me.

comment:7 Changed 10 years ago by bill

Summary: GridContainer resize problem with StackContainer child[patch] [regression] GridContainer resize problem with StackContainer child

Do you have a CLA? I looked up your user info but it just says that your name is Moogle Moogle.

comment:8 Changed 10 years ago by Mathevet julien

I have send it today .... But I couldn't modify my user first on trac or on dojotoolkit website... My name is Julien Mathevet

Changed 10 years ago by Mathevet julien

Attachment: patchGridContainer.patch added

comment:9 Changed 10 years ago by bill

Milestone: tbd1.4
Summary: [patch] [regression] GridContainer resize problem with StackContainer child[patch] [cla] [regression] GridContainer resize problem with StackContainer child

comment:10 Changed 10 years ago by dante

Status: newassigned

comment:11 Changed 10 years ago by dante

Hmmm, so I'm not seeing anything useful in the tabContainer test case attached. The patch applies fine, and looks ok but to be honest I'm not clear on what this is fixing.

comment:12 Changed 10 years ago by Mathevet julien

Yes I know it's not a good example but there few widget that extends LayouWidget?. The main problem is startup order. In current GridContainer it tries to start many time childs without any reasons... And then my child start before the grid. Then there column aren't set up an child take wrong size. And there is also conceptual problem.

And it fixes also getChildren method: #10244. So the resize method doesn't work.

Ok I will try to add a more clear example.

Current code:

startup:function(){
    // Start childs and get wrong size
    this.inherited(arguments);
    // create columns
    this._createCells();
		
    // start again childs
     dojo.forEach(this.getChildren(), function(child){
	if(!child.started && !child._started){
           child.startup();
	}
     });
     // move child in table grid
     if(this.usepref !== true){
			this[(this.isAutoOrganized ? "_organizeServices" : "_organizeServicesManually")]();
		}else{ 
			//console.info("GridContainer organised by UserPref");
			return;
		}
                // create grids
		this.init();
	},

comment:13 Changed 10 years ago by Mathevet julien

If you look at _Container, startup says:

// summary:
			//		Called after all the widgets have been instantiated and their
			//		dom nodes have been inserted somewhere under dojo.doc.body.
			//
			//		Widgets should override this method to do any initialization
			//		dependent on other widgets existing, and then call
			//		this superclass method to finish things off.
			//
			//		startup() in subclasses shouldn't do anything
			//		size related because the size of the widget hasn't been set yet.

So this.inherited(arguments); need to be call at end of child startup. why dev has done that. Because getChildren method return only node are directly under ContainerNode. So after move childs inside table grid, getChildren method return an empty array. So childs aren't started. But in current code child are started before GridContainer finish to be built. In svn tests they are only ContentPane child. But !_LayoutWidget child doesn't manage resize method like ContentPane and they take size of ContainerNode and not of grid cell.

There is also confusing method between addService and addChild. If you do addChild after startup it will not be insertted inside grid. And if you do addService before startup child will be never startup.

So this patch fix getChildren, startup, addChild in repecting parents conception.

comment:14 Changed 10 years ago by dante

Resolution: fixed
Status: assignedclosed

(In [20733]) fixes #10241, #10244, #7469, #9271 - addCHild/startup regression fix from moogle w/slight style modifictions. other bugs/issues need new tickets and patches, and prefereably additional test cases \!strict

Note: See TracTickets for help on using tickets.