Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10872 closed defect (fixed)

dojox.grid.TreeGrid getItem() returns null for three level path

Reported by: izikz Owned by: Bryan Forbes
Priority: blocker Milestone: 1.5.1
Component: DojoX Grid Version: 1.4.0
Keywords: dojox.grid.TreeGri getItem null three level Cc:
Blocked By: Blocking:

Description

getItem() does not work properly in dojox.grid.TreeGrid?. For example:

dojox/grid/tests/test_treegrid_model.html

...

var grid2 = new dojox.grid.TreeGrid?({

treeModel: treeModel2,

structure: layout,

defaultOpen: true

}, 'programmatic_grid');

grid2.startup();

dojo.connect(window, "onresize", grid2, "resize");

var item1 = grid2.getItem('0/0'); returns an object

var item2 = grid2.getItem('0/0/0'); returns null

Attachments (1)

TreeGrid.js (26.8 KB) - added by Yves De Bruyne 9 years ago.
With hotfix for one-category treestore

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by izikz

Correction: the example should be var item2 = grid2.getItem('0/1/0'); returns null

comment:2 Changed 10 years ago by Douglas Hays

Owner: changed from bryanforbes to Bryan Forbes

comment:3 Changed 9 years ago by Yves De Bruyne

This error seems to be in getItem from TreeGrid?.js

The existing codes makes the assumption that cf[i] (array with child attribute names) is defined for every level of the path. In most real-life examples I've seen though, we define only one value and stick to it. e.g. cf=children?.

This explains why it fails at level 3 (and later) levels, as soon as it gets an indexOutOfBounds on cf[].

I'm not quite sure what the correct way of solving this is, but the uploaded code will fix the problem if you use only one childrenAttrs in your source.

if(cf){
	for(var i = 0; i < idx.length - 1 && itm; i++){
		if(cf[0]){
			itm = (s.getValues(itm, cf[0])||[])[idx[i + 1]];
		}else{
			itm = null;
		}
	}
}

Changed 9 years ago by Yves De Bruyne

Attachment: TreeGrid.js added

With hotfix for one-category treestore

comment:4 Changed 9 years ago by Yves De Bruyne

it seems that both #10945 and #10823 would be duplicates of this bug.

comment:5 Changed 9 years ago by wirelessdreamer

I am experiencing this problem as well, another simple test is load dojox/grid/tests/test_treegrid_model.html then select Nairobi and enter this in a debugger: grid2.selection.getSelected(); nothing gets returned, which makes using a tree grid to work with data pretty useless.

comment:6 Changed 9 years ago by samduke474

I am also experiencing this problem.

Looking at TreeStoreModel? getChildren, it seems that for a given parent item, all the childrenattrs are used to get children for that item:

// get children of specified item
	        var childItems = [];
	        for (var i = 0; i < this.childrenAttrs.length; i++) {
	            var vals = store.getValues(parentItem, this.childrenAttrs[i]);
	            childItems = childItems.concat(vals);
	        }

So to be consistent in this case would require a search of every children attribute of every possible parent and then for each result search every children attribute. Perhaps only set a final itm value if i is at its final value (ie we are at the end of idx) and search all tree possibilities until that is set.

If the for loop is to be kept as it is then at every level of idx an array of possible parents (or grandparents...) to the desired final item must be carried forward. Perhaps use the Treestoremodel getchildren method if we have a TreeModel??

I dont think the above solution ideas work though as it doesn't make much sense to have two types of children (eg 'children' and 'kids') at each level and only specify one number per level. (eg 1/5/3/4, does that mean we need to find the 1st 'child' and the 1st 'kid' and then inside each of those find the 5th 'child' and the 5th 'kid'?) If cf.length is greater than 1 I think an error should be thrown or a different input to this function should be required. Maybe 1:1/0:5/0:3/1:4 where the number before the colon identifies the childrenAttr to use at that level.

comment:7 Changed 9 years ago by Seldarly

I've fixed it like this:

if(cf){
            for(var i=0; i<idx.length-1; i++) {
                for(var j=0; j<cf.length; j++) {
                    itm = (s.getValues(itm, cf[j])||[])[idx[i + 1]];
                    if(itm != null) {
                        break;
                    }
                }
            }
        }

Works fine for me.

comment:8 Changed 9 years ago by Bryan Forbes

(In [23259]) Fixes TreeGrid?.getItem(). refs #10872

comment:9 Changed 9 years ago by Bryan Forbes

(In [23260]) Backported fix for TreeGrid?.getItem() from trunk. refs #10872

comment:10 Changed 9 years ago by Bryan Forbes

Milestone: tbd1.5.1
Resolution: fixed
Status: newclosed

comment:11 Changed 9 years ago by Bryan Forbes

(In [23324]) Changed TreeGrid?.getItem() to work differently between models and grouped data. refs #10872

comment:12 Changed 9 years ago by Bryan Forbes

(In [23326]) Backport of fix from trunk. refs #10872

Note: See TracTickets for help on using tickets.