PDA

View Full Version : [FIXED][3.*,2.*]Tree keyboard navigation visits hidden nodes.



Animal
4 Aug 2009, 2:28 AM
If TreeNodeUI.hide is called, that node should not be visitable by keyboard navigation.

The follownig fixes are needed. Tested and working here.



Ext.override(Ext.tree.DefaultSelectionModel, {
/**
* Select a node.
* @param {TreeNode} node The node to select
* @return {TreeNode} The selected node
*/
select : function(node, /* private*/ selectNextNode){

// If node is hidden, select the next node in whatever direction was being moved in.
if (!Ext.fly(node.ui.wrap).isVisible() && selectNextNode) {
return selectNextNode.call(this, node);
}
var last = this.selNode;
if(node == last){
node.ui.onSelectedChange(true);
}else if(this.fireEvent('beforeselect', this, node, last) !== false){
if(last){
last.ui.onSelectedChange(false);
}
this.selNode = node;
node.ui.onSelectedChange(true);
this.fireEvent("selectionchange", this, node, last);
}
return node;
},

/**
* Selects the node above the selected node in the tree, intelligently walking the nodes
* @return TreeNode The new selection
*/
selectPrevious : function(/* private */ s){
if(!(s = s || this.selNode || this.lastSelNode)){
return null;
}
var ps = s.previousSibling;
if(ps){
if(!ps.isExpanded() || ps.childNodes.length < 1){
return this.select(ps, this.selectPrevious);
} else{
var lc = ps.lastChild;
while(lc && lc.isExpanded() && lc.childNodes.length > 0){
lc = lc.lastChild;
}
return this.select(lc, this.selectPrevious);
}
} else if(s.parentNode && (this.tree.rootVisible || !s.parentNode.isRoot)){
return this.select(s.parentNode, this.selectPrevious);
}
return null;
},

/**
* Selects the node above the selected node in the tree, intelligently walking the nodes
* @return TreeNode The new selection
*/
selectNext : function(/* private */ s){
if(!(s = s || this.selNode || this.lastSelNode)){
return null;
}
if(s.firstChild && s.isExpanded()){
return this.select(s.firstChild, this.selectNext);
}else if(s.nextSibling){
return this.select(s.nextSibling, this.selectNext);
}else if(s.parentNode){
var newS = null;
s.parentNode.bubble(function(){
if(this.nextSibling){
newS = this.getOwnerTree().selModel.select(this.nextSibling, this.selectNext);
return false;
}
});
return newS;
}
return null;
}
});

VinylFox
4 Aug 2009, 8:49 AM
Verified problem on trunk rev 4961

Proposed override tested and working on FF3.5 & IE7 in Vista.

Tested using the tree/reorder example, keyboard nav works fine to start with, but after you hide a node...


Ext.getCmp('ext-comp-1001').getNodeById('src/adapter').getUI().hide();

Before applying the override FF3.5 seems to break the key nav entirely, while IE7 just does a 'ghost' selection.

After the override is applied, key navigation works as expected.

Animal
4 Aug 2009, 2:02 PM
To clarify, "FF3.5 seems to break the key nav entirely, while IE7 just does a 'ghost' selection." is still hapenning with my proposed fix added?

zhegwood
4 Aug 2009, 2:21 PM
I did this and it worked in FF3 & IE7 about 3 or 4 months ago in 2.x. I haven't tested it since, and I'm sure there's a better way to do it, but this works AFAIK.



Ext.override(Ext.tree.DefaultSelectionModel,{

selectNext : function(node){
if (!node) {
node = this.selNode || this.lastSelNode;
if(!node){
node = this.tree.getRootNode();
}
}
this.tree.getTreeEl().focus();
if (node.firstChild) {
if (!node.firstChild.hidden) {
return this.select(node.firstChild);
} else {
this.selectNext(node.firstChild);
}
} else if (node.nextSibling) {
if (!node.nextSibling.hidden) {
return this.select(node.nextSibling);
} else {
this.selectNext(node.nextSibling);
}
} else {
var newS = null;
node.parentNode.bubble(function(){
if(this.nextSibling){
newS = this.nextSibling;
} else {
return false;
}
});
if (!newS.hidden) {
return this.select(newS);
} else {
this.selectNext(newS);
}
}
},

selectPrevious : function(node){
if (!node) {
node = this.selNode || this.lastSelNode;
}
if(node === this.tree.getRootNode()){
return null;
}
this.tree.getTreeEl().focus();

if (node.previousSibling) {
if (node.previousSibling.childNodes.length > 0){
if (!node.previousSibling.lastChild.hidden) {
return this.select(node.previousSibling.lastChild);
} else {
this.selectPrevious(node.previousSibling.lastChild);
}
} else {
if (!node.previousSibling.hidden) {
return this.select(node.previousSibling);
} else {
this.selectPrevious(node.previousSibling);
}
}
} else if (node.parentNode) {
if (!node.parentNode.hidden) {
return this.select(node.parentNode);
} else {
this.selectPrevious(node.parentNode);
}
}
}
});

Animal
5 Aug 2009, 2:24 AM
That code did disappear the selection if a non-leaf node was hidden.

After further testing, this works better:



Ext.override(Ext.tree.DefaultSelectionModel, {
/**
* Select a node.
* @param {TreeNode} node The node to select
* @return {TreeNode} The selected node
*/
select : function(node, /* private*/ selectNextNode){

// If node is hidden, select the next node in whatever direction was being moved in.
if (!Ext.fly(node.ui.wrap).isVisible() && selectNextNode) {
return selectNextNode.call(this, node);
}
var last = this.selNode;
if(node == last){
node.ui.onSelectedChange(true);
}else if(this.fireEvent('beforeselect', this, node, last) !== false){
if(last){
last.ui.onSelectedChange(false);
}
this.selNode = node;
node.ui.onSelectedChange(true);
this.fireEvent("selectionchange", this, node, last);
}
return node;
},

/**
* Selects the node above the selected node in the tree, intelligently walking the nodes
* @return TreeNode The new selection
*/
selectPrevious : function(/* private */ s){
if(!(s = s || this.selNode || this.lastSelNode)){
return null;
}
var ps = s.previousSibling;
if(ps){
if(!ps.isExpanded() || ps.childNodes.length < 1){
return this.select(ps, this.selectPrevious);
} else{
var lc = ps.lastChild;
while(lc && lc.isExpanded() && Ext.fly(lc.ui.wrap).isVisible() && lc.childNodes.length > 0){
lc = lc.lastChild;
}
return this.select(lc, this.selectPrevious);
}
} else if(s.parentNode && (this.tree.rootVisible || !s.parentNode.isRoot)){
return this.select(s.parentNode, this.selectPrevious);
}
return null;
},

/**
* Selects the node above the selected node in the tree, intelligently walking the nodes
* @return TreeNode The new selection
*/
selectNext : function(/* private */ s){
if(!(s = s || this.selNode || this.lastSelNode)){
return null;
}
if(s.firstChild && Ext.fly(s.ui.wrap).isVisible() && s.isExpanded()){
return this.select(s.firstChild, this.selectNext);
}else if(s.nextSibling){
return this.select(s.nextSibling, this.selectNext);
}else if(s.parentNode){
var newS = null;
s.parentNode.bubble(function(){
if(this.nextSibling){
newS = this.getOwnerTree().selModel.select(this.nextSibling, this.selectNext);
return false;
}
});
return newS;
}
return null;
}
});

VinylFox
5 Aug 2009, 5:30 AM
To clarify, "FF3.5 seems to break the key nav entirely, while IE7 just does a 'ghost' selection." is still hapenning with my proposed fix added?

Sorry, I see how my statement was confusing. I was describing the problem that exists before your override is applied.

Your override worked perfectly for me.

Animal
5 Aug 2009, 5:31 AM
Ah well, it made me look again, and I did find a problem which is fixed in my latest post.

evant
5 Aug 2009, 9:08 PM
Fixed in SVN, thanks.