Opened 11 years ago

Closed 11 years ago

#8028 closed defect (fixed)

[patch] [cla] Tree: incorrect behaviour handling END keystroke

Reported by: Joseph Scheuhammer Owned by:
Priority: high Milestone: 1.3
Component: Dijit Version: 1.2.0
Keywords: tree keyboard handling Cc: Becky Gibson, davidb
Blocked By: Blocking:

Description

dijit.Tree's _onEndKey() method incorrectly handles an END keystroke. The result is a no-op, when focus should move to the last visible open tree node.

Attachments (1)

8028.patch (392 bytes) - added by Joseph Scheuhammer 11 years ago.

Download all attachments as: .zip

Change History (6)

Changed 11 years ago by Joseph Scheuhammer

Attachment: 8028.patch added

comment:1 Changed 11 years ago by Joseph Scheuhammer

The bug is the first line of _onEndKey():

    var node = this;

"this" is not a dijit._TreeNode, and subsequent calls to tree node methods do not work. The line should read:

    var node = this.rootNode;

The attached patch, "8028.patch", does just that.

comment:2 Changed 11 years ago by bill

Perhaps this._getRootOrFirstNode() is better than this.rootNode (remember that the rootNode may be hidden; see second example in test_Tree.html), but also, the code looks wrong to me for a case like:

+ a
    +  a1
    +  a2
+ b
+ c
+ d

Won't the current code (starting at "a") end up at "a2" instead of "d"?

comment:3 in reply to:  2 Changed 11 years ago by Joseph Scheuhammer

Replying to bill:

Perhaps this._getRootOrFirstNode() is better than this.rootNode (remember that the rootNode may be hidden; see second example in test_Tree.html) ...

I tried that and it didn't work. Using the 2nd test in test_Tree.html as an example, the first node is "Africa" and the search for the last visible node is therefore confined to the Africa branch. That won't find the last top level node (South America) if that is the last visible node. In your example, it ends up at "a2", not "d".

Even if the root node is not displayed, it still exists, and is the place to start the search for the last visible node. Note that for the invisible root case, the root node is always in an expanded state -- there is no chance that the search will end on the root node.

comment:4 Changed 11 years ago by Joseph Scheuhammer

I suppose another possibility is to add a _getRootOrLastNode() function:

    _getRootOrLastNode: function(){
       // summary: get root if displayed, or last visible top level node
        if(this.showRoot){
            return this.rootNode;
        }else{
            var c = this.rootNode.getChildren();
            return c[c.length-1];
        }
    },

comment:5 Changed 11 years ago by bill

Milestone: tbd1.3
Resolution: fixed
Status: newclosed

Fixed in [15636].

Note: See TracTickets for help on using tickets.