PDA

View Full Version : [FIXED-423][3.1r5850] Can't tab to next row on EditorGrid



Gjslick
10 Jan 2010, 6:47 PM
Ext version tested:


Ext 3.1.0 rev. 5850


Adapter used:


ext


css used:


only default ext-all.css


Browser versions tested against:


IE7
IE8
FF 3.5.7 (firebug 1.4.5 installed)


Operating System:


WinXP Pro



Description:
If there is only one editable column on an editor grid, tabbing to the next editable cell (which is on the next row, in the same column) causes editing mode to be lost. I mention "in the same column" because I believe that this fact is the cause of the issue.

What happens is that when the tab key is pressed, the new cell starts to become editable, but then immediately switches off edit mode.

---

To be more descriptive:

Because the tabbing is confined to the same column, I think there is a "race condition" (per se) in the single editor. The onkeydown of the tab key moves the editing to the next row, but then onblur fires from the editor right after that, which is causing editing to stop (via EditorGridPanel::onEditComplete). This does not happen if there is more than one editable column, as the onblur of one editor would not affect the next.

Simplified steps that the code is taking (in order):


The onkeydown of the tab key moves editing to the next editable cell (which is in the same column, and therefore using the same actual editor instance).
That same editor's onblur fires, and causes editing to be stopped.


This is not only a problem in single column editor grids, but the reason that I came across this is because it is currently impossible to create a CellSelectionModel where the tab key moves the selection down the grid, instead of over (due to this same issue).



Test Case:
This is a simple test case to see the behavior, but click the link below it with firebug enabled to see the console debugging output that helped me narrow down this problem.


Ext.onReady( function() {

var editorGrid = new Ext.grid.EditorGridPanel( {
renderTo: Ext.getBody(),
width: 400,
height: 400,

title: "Editor Grid",

store: new Ext.data.ArrayStore( {
fields: [ 'name', 'value' ],
data: [
[ 'A', '' ],
[ 'B', '' ],
[ 'C', '' ],
[ 'D', '' ]
]
} ),

columns: [
{
header: 'Name',
dataIndex: 'name'
},{
header: 'Value',
dataIndex: 'value',
editor: new Ext.form.TextField()
}
],

clicksToEdit: 1,
columnLines: true
} );

} ); // eo onReady



See this URL : http://www.edsoftdesign.com/extTests/editorGrid/
(Load with firebug enabled to see debugging output.)


Steps to reproduce the problem:


Load the link with firebug enabled.
Click in an editable cell (not the bottom one), and then hit the tab key.
A few firebug console logs will come up, with two traces in the midst. Just read the console logs through one by one and you'll understand. The last two logs are of the most interest, where editing starts, and then onEditComplete() runs right after that to stop the editing (from the onblur event, shown in the trace right after it).

The traces are each created from EditorGridPanel::onEditComplete, and show the html event that caused onEditComplete() to run. The second one, run by onblur of the textfield, is I believe the cause of the problem.



Debugging already done:


See above



Possible fix:


This is not the real fix for this issue, but it can be worked around if the onBlur functionality is skipped. However, the condition for the onBlur functionality to be skipped is not correctly tested for. The docs state that the allowBlur config defaults to false (really defaults to undefined), but the onBlur method is currently calling this.completeEdit() when allowBlur is false, instead of true.

Hence, setting Ext.Editor.prototype.allowBlur to true allows it to start working correctly, by not running the onBlur's this.completeEdit call. Here's where it is in the code:


// Ext.Editor (src/widgets/Editor.js, line 349 of rev. 5850+)
onBlur : function(){
if(this.allowBlur !== true && this.editing) { // Should be: ===
this.completeEdit();
}
}
You'll have to default allowBlur to true on the prototype to maintain the current behavior though if this is fixed. Also, it's usually desirable to have the allowBlur functionality turned on, so a more "root cause" fix for this issue is still needed.



Anyway, thanks, and let me know if you need any more information.

-Greg

jsakalos
11 Jan 2010, 3:58 AM
+1

acarapetis
18 Jan 2010, 8:25 PM
I ran into this bug on an editorgrid with many editable columns... I'm having trouble diagnosing the exact conditions when it happens, but the symptoms are similar: The new field is focused for a fraction of a second, then focus to the grid is lost entirely.

Gjslick's patch fixed my issue.

Gjslick
18 Jan 2010, 10:46 PM
Hey acarapetis, can you put up a test case or link where you see the problem happening with multiple editable columns?

Also, are you using 3.1.0? If I recall correctly, I think that there was a problem before 3.1.0 where tabbing could make the entire grid itself lose focus. However, the problem that I'm experiencing is that just the editing is lost, but the cell remains selected and the grid is still navigable with tab.

acarapetis
19 Jan 2010, 2:38 PM
Also, are you using 3.1.0? If I recall correctly, I think that there was a problem before 3.1.0 where tabbing could make the entire grid itself lose focus. However, the problem that I'm experiencing is that just the editing is lost, but the cell remains selected and the grid is still navigable with tab.

I upgraded to 3.1.0 and still have the same issue - the whole grid is losing focus. The patch


Ext.override(Ext.Editor, {
onBlur: function() {
if (this.allowBlur === true && this.editing) {
this.completeEdit();
}
}
});
still fixes it.

I'll see if I can reduce it to a simple example for you; I'm having trouble pinpointing the conditions under which it occurs.

JamesC
19 Jan 2010, 2:40 PM
Pretty sure this is a dup of http://www.extjs.com/forum/showthread.php?t=88086

Gjslick
19 Jan 2010, 3:14 PM
@James, I think that they are describing roughly the same issue in that thread, but I believe that I have found the root cause of the issue in my bug report here.

Your one post in that thread is definitely the same issue that I have found. Read my description, and check out the test page (the link) that I provided with firebug enabled. The reason that things look ok when your code hits your breakpoint is that the editor's onBlur() hasn't run yet by then.


@acarapetis, The override kind-of fixes it, in that it's the blur functionality that's causing the issue in the first place. However, if we want to the grid to work correctly, and have the blur functionality, we need a better fix.

Jamie Avins
19 Jan 2010, 4:06 PM
Same issue, but I prefer this example as well.

Jamie Avins
19 Jan 2010, 5:44 PM
Fixed in svn 5925.

Gjslick
19 Jan 2010, 5:54 PM
Nice, thanks Jamie.

And James you were correct, it was the same issue. At first glance of that thread and seeing the use of the RowSelectionModel, I wasn't 100% sure. But after trying the example, and plugging it in with the console logging overrides that I originally used, it definitely was.

Thanks guys,

Greg

JamesC
20 Jan 2010, 2:06 PM
Jamie, will this be in 3.1.1?

Jamie Avins
20 Jan 2010, 9:30 PM
Yes

GustavR
22 Jan 2010, 9:41 AM
I just tried this fix out:
Updated to HEAD and tried this in an EditorGrid in my software, curiously it works if I open an editor and then close the editor again with ESC, reopen it and then press TAB, it jumps directly were I want it to jump.
But if I open an editor (after fresh reload) and just press TAB it jumps out of the editor and nothing more happens.
After doing this "ESC-trick" it works from then on.
I'm using RowSelectionModel.

I can try to prepare an example if wanted.

Jamie Avins
25 Jan 2010, 7:37 AM
Sure, more cases always help. I'll put this back as an inforeq for now.

GustavR
25 Jan 2010, 3:19 PM
Ok, I did an override to have TAB to do the same that ENTER does in an EditorGrid (override of "onEditorKey"), but that makes no big difference in the fact that there is a bug, but with the fact, that the bug is with the ENTER key.
Here is my example, that I found: Just add "sm: new Ext.grid.RowSelectionModel()," to "\examples\grid\totals.js" (inside of the grid of course).
Then open the example and try out the ENTER-key.
In the DateFild it works just great, in all the other fields you need to use the ESC-trick. Tried this out with HEAD-revision.
Hope this helps a bit.

mystix
9 Feb 2010, 8:53 AM
i think your test case should be


sm: new Ext.grid.RowSelectionModel({
moveEditorOnEnter: true // only works on TriggerField editors
}),

instead. it works correctly otherwise.