PDA

View Full Version : [CLOSED] [ 3.3.0 ] KeyNav enabled in the viewport when showing modal windows



radubrehar
21 Dec 2010, 6:36 AM
Hi,

I have found an important bug, here is the description:

When showing a modal window, using keyboard navigation it is possible to access buttons/menus/components from under the modal dialog, and activate them with the "space" key.

Expected behaviour: using keyboard nav, the user should only be able to navigate in components of the modal window.

Here is the code to prove it, together with a partial fix for it - see the comment to the fix:



Ext.onReady(function(){

var newWindow = function(){
Ext.Msg.alert('Status', 'Changes saved successfully.');
}

var w = new Ext.Viewport({

frame: true,
autoScroll: true,
defaults: {
border: false,
bodyBorder: false,
xtype: 'button'
},
items: [{
text: 'test',
handler: newWindow
},{
text: 'test1',
handler: newWindow
}]
});

w.show();

new Ext.Window({
height: 200,
width: 300,
modal: true,
buttons: [{text: 'ok', focus: true, handler: function(){ Ext.Msg.alert('Status', 'Hey.');} }]
}).show();

//HERE IS THE FIX - comment the 'return' in order to have the fix working
return;
new Ext.KeyNav(Ext.getBody(), {
"tab" : function(e){
var targetEl = Ext.get(e.target),
activeWindow = Ext.WindowMgr.getActive();

if (activeWindow.modal){
console.log(targetEl, 'return ', !!activeWindow.el.child('#'+targetEl.id));
return !!activeWindow.el.child('#'+targetEl.id);
}
console.log(targetEl, 'return true');
return true;
}
});

})

Now the problem with the fix is that I can still navigate to the first component outside the modal window. From there on, the user cannot navigate to any other component.

What would be needed is to get the next keynav target and this would make the fix perfect.

Any feedback on this?

Thank you

Condor
21 Dec 2010, 7:10 AM
This is a know issue and it is not considered a bug.

Other suggested workarounds are:
1. Disabling all other inputs, buttons etc. when showing a modal window (too time consuming IMHO).
2. Using the focusout event to avoid giving the focus to any element outside a modal window (that was my suggestion).

radubrehar
22 Dec 2010, 2:48 AM
Hi Condor,

after a bit more thinking I have come up with a solution to this, based on the above partial fix. This time it's a complete fix and as far as I have tested, it works smoothly.
I based my assumption on the fact that in the browser, only 'button' and 'input' elements can receive focus - if there are some others, please let me know.

So here is the code


Ext.onReady(function(){

function newWindow(){
Ext.Msg.alert('test','not good');
}

var w = new Ext.Viewport({

frame: true,
autoScroll: true,
defaults: {
border: false,
bodyBorder: false,
xtype: 'button'
},
items: [{
text: 'test',
handler: newWindow
},{
text: 'test1',
handler: newWindow
}]
});

w.show();

new Ext.Window({
height: 200,
width: 300,
modal: true,
layout: 'fit',
items: {
border: false,
layout: 'form',
padding: '5px',
defaults: { xtype: 'textfield' },
items: [
{ fieldLabel: 'Name'},
{ fieldLabel: 'Age'}
]
},
buttons: [
{ text: 'ok', focus: true },
{ text: 'cancel' }
]
}).show();

new Ext.KeyNav(Ext.getBody(), {
"tab" : function(e){
//return true in order to continue tab navigation
//and false if the element that would be receiving focus is outside of the modal window

var targetDom = e.target,
activeWindow = Ext.WindowMgr.getActive(),
activeWindowEl = activeWindow.el;

if (activeWindow.modal){

var returnValue = !!activeWindowEl.contains(targetDom);
console.log('target is ', targetDom);

if (!returnValue){
//we should not reach this code, as the user should never be able to be on an element outside the activeWindow
//and hit 'tab' there - yet leave this here for safety
console.log('found target outside the window');
return false;
} else {

var composite = activeWindowEl.select('button, input'), //enumerate here elements that can receive focus
index = composite.indexOf(targetDom),
toAdd = e.shiftKey ? -1 : 1; //detect SHIFT + TAB;

//check whether the active window contains the next (or prev in case of SHIFT+TAB) focus element that comes after targetDom
if (index != -1 && activeWindowEl.contains(composite.item(index+toAdd))){
return true;
}

return false;
}
}

return true;
}
});

})

Hope it will help others as well and hope it will be considered in a next minor version of Ext!
Thanks.

Condor
22 Dec 2010, 4:19 AM
Everything with a tabIndex can get the focus.

This includes <input>, <button>, <select>, <textarea>, <a>, anything that is contentEditable or designMode="on", but also other elements like <div> can have a tabIndex and an onclick handler!

radubrehar
22 Dec 2010, 4:43 AM
Thank you for the list :) I forgot the others, and didn't know about contentEditable, designMode and tabIndex things.
Yet in an common ExtJS application where one uses components mainly instead of hand crafting complex DOM structures, I don't think there is the case with contentEditable and the rest. Am I doing a correct assumption?

So the correct list would be :

var composite = activeWindowEl.select('button, input, select, textarea, a');

and could easily add the others - worths the overhead?


var composite = activeWindowEl.select('button, input, select, textarea, a, [contentEditable], [designMode="on"], [tabIndex]');

Do you think this is a more realistic implementation?

Thanks for your input.

Condor
22 Dec 2010, 4:53 AM
You're ignoring the fact that tabIndex modifies the tab order.

Let's say the focus is on the input with the highest tabIndex, but this is not the last input in the form. Pressing tab will move the focus outside the window, but your code assumes the next input inside the window gets the focus.

radubrehar
22 Dec 2010, 9:51 AM
Hm... I see what you mean. I'll look for a solution to that as well -in any case, it's quite a rare situation.
Well, even in that case, the user can navigate only to the first element (in tab-navigation order) outside the modal window, and then on the next press of the 'tab' key, it will go through


if (!returnValue){
return false
}

Which will stop the user to further navigate to other elements.

Thanks a lot for the feedback Condor!