diff --git a/editor.js b/editor.js index 6578627de..acdaace43 100644 --- a/editor.js +++ b/editor.js @@ -12,10 +12,6 @@ define([ "dojo/_base/sniff" ], function(kernel, lang, arrayUtil, Deferred, on, aspect, has, query, Grid, put){ -// Variables to track info for cell currently being edited -// (active* variables are for editOn editors only) -var activeCell, activeValue, activeOptions, focusedCell; - function updateInputValue(input, value){ // common code for updating value of a standard input input.value = value; @@ -111,15 +107,16 @@ function setPropertyFromEditor(grid, cmp, triggerEvent) { column = cell.column, value, id, - editedRow; + editedRow, + activeCell = grid._activeCell; if(!cmp.isValid || cmp.isValid()){ value = setProperty(grid, cell, - activeCell ? activeValue : cmp._dgridLastValue, + activeCell ? grid._activeValue : cmp._dgridLastValue, dataFromEditor(column, cmp), triggerEvent); if(activeCell){ // for editors with editOn defined - activeValue = value; + grid._activeValue = value; }else{ // for always-on editors, update _dgridLastValue immediately cmp._dgridLastValue = value; } @@ -238,7 +235,7 @@ function createSharedEditor(column, originalRenderCell){ keyHandle; function blur(){ - var element = activeCell; + var element = grid._activeCell; focusNode.blur(); if(typeof grid.focus === "function"){ @@ -276,13 +273,13 @@ function createSharedEditor(column, originalRenderCell){ // Clear out the rest of the cell's contents, then re-render with new value. while(i--){ put(parentNode.firstChild, "!"); } Grid.appendIfNode(parentNode, column.renderCell( - column.grid.row(parentNode).data, activeValue, parentNode, - activeOptions ? lang.delegate(options, activeOptions) : options)); + column.grid.row(parentNode).data, grid._activeValue, parentNode, + grid._activeOptions ? lang.delegate(options, grid._activeOptions) : options)); // Reset state now that editor is deactivated; // reset focusedCell as well since some browsers will not trigger the // focusout event handler in this case - activeCell = activeValue = activeOptions = focusedCell = null; + grid._focusedCell = grid._activeCell = grid._activeValue = grid._activeOptions = null; } function dismissOnKey(evt){ @@ -292,7 +289,7 @@ function createSharedEditor(column, originalRenderCell){ if(key == 27){ // escape: revert + dismiss reset(); - activeValue = cmp._dgridLastValue; + grid._activeValue = cmp._dgridLastValue; blur(); }else if(key == 13 && column.dismissOnEnter !== false){ // enter: dismiss // FIXME: Opera is "reverting" even in this case @@ -338,8 +335,8 @@ function showEditor(cmp, column, cellElement, value){ cmp._dgridLastValue = value; // if this is an editor with editOn, also update activeValue // (activeOptions will have been updated previously) - if(activeCell){ - activeValue = value; + if(grid._activeCell){ + grid._activeValue = value; // emit an event immediately prior to placing a shared editor on.emit(cellElement, "dgrid-editor-show", { grid: grid, @@ -363,7 +360,7 @@ function edit(cell) { // input/widget when the cell editor is focused. // If the cell is not editable, returns null. - var row, column, cellElement, dirty, field, value, cmp, dfd; + var row, column, cellElement, dirty, field, value, cmp, dfd, activeCell, node; if(!cell.column){ cell = this.cell(cell); } if(!cell || !cell.element){ return null; } @@ -371,6 +368,7 @@ function edit(cell) { column = cell.column; field = column.field; cellElement = cell.element.contents || cell.element; + activeCell = column.grid._activeCell; if((cmp = column.editorInstance)){ // shared editor (editOn used) if(activeCell != cellElement){ @@ -383,6 +381,15 @@ function edit(cell) { if(!column.canEdit || column.canEdit(cell.row.data, value)){ activeCell = cellElement; + // In some browsers, moving a DOM node causes a blur event to fire which is not + // the best time for the blur handler to fire. Force the issue by blurring the + // editor now. + node = cmp.domNode || cmp; + if(node.offsetWidth){ + // The editor is visible. Blur it. + node.blur(); + } + showEditor(column.editorInstance, column, cellElement, value); // focus / blur-handler-resume logic is surrounded in a setTimeout @@ -431,15 +438,16 @@ return function(column, editor, editOn){ grid.edit = edit; listeners.push(on(grid.domNode, '.dgrid-input:focusin', function () { - focusedCell = grid.cell(this); + grid._focusedCell = grid.cell(this); })); focusoutHandle = grid._editorFocusoutHandle = on.pausable(grid.domNode, '.dgrid-input:focusout', function () { - focusedCell = null; + grid._focusedCell = null; }); listeners.push(focusoutHandle); listeners.push(aspect.before(grid, 'removeRow', function (row) { + var focusedCell = grid._focusedCell; row = grid.row(row); if (focusedCell && focusedCell.row.id === row.id) { // Pause the focusout handler until after this row has had @@ -455,7 +463,8 @@ return function(column, editor, editOn){ } })); listeners.push(aspect.after(grid, 'insertRow', function (rowElement) { - var row = grid.row(rowElement); + var row = grid.row(rowElement), + focusedCell = grid._focusedCell; if (focusedCell && focusedCell.row.id === row.id) { grid.edit(grid.cell(row, focusedCell.column.id)); } @@ -518,12 +527,12 @@ return function(column, editor, editOn){ // TODO: Consider using event delegation // (Would require using dgrid's focus events for activating on focus, // which we already advocate in README for optimal use) - + var grid = column.grid; if(!options || !options.alreadyHooked){ // in IE<8, cell is the child of the td due to the extra padding node on(cell.tagName == "TD" ? cell : cell.parentNode, editOn, function(){ - activeOptions = options; - column.grid.edit(this); + grid._activeOptions = options; + grid.edit(this); }); } diff --git a/test/intern/plugins/editor.js b/test/intern/plugins/editor.js index 4b8a0a616..1c67bfd93 100644 --- a/test/intern/plugins/editor.js +++ b/test/intern/plugins/editor.js @@ -93,6 +93,8 @@ define([ assert.isUndefined(results[3], "canEdit should not have been called yet for editOn editor (item 3)"); + // Note: The "Data 2" column's canEdit method always returns false so none of the following + // grid.edit calls will return a promise and not editor with receive focus. grid.edit(grid.cell(1, "data2")); assert.isUndefined(results[1], "canEdit should not have been called yet for editOn editor (item 1)"); @@ -118,16 +120,15 @@ define([ "canEdit should have been called for editOn editor (item 3)"); }); - - test.test("canEdit: suppress on false", function () { - var rowIndex, + function testCanEdit(dfd, columnId){ + var rowCount, cell, matchedNodes; function canEdit(data) { return data.order % 2; } - + grid = new OnDemandGrid({ columns: { order: "step", @@ -148,38 +149,45 @@ define([ document.body.appendChild(grid.domNode); grid.startup(); grid.renderArray(testOrderedData); + rowCount = testOrderedData.length; - for (rowIndex = 0; rowIndex < testOrderedData.length; rowIndex++) { - // Test always-on editors - cell = grid.cell(rowIndex, "name"); - grid.edit(cell); - matchedNodes = query("input", cell.element); - - if (canEdit(cell.row.data)) { - assert.strictEqual(1, matchedNodes.length, - "Cell with canEdit=>true should have an editor element"); - } - else { - assert.strictEqual(0, matchedNodes.length, - "Cell with canEdit=>false should not have an editor element"); - } + function testRow(rowIndex){ + var editPromise; - // Test non-always-on editors - cell = grid.cell(rowIndex, "description"); - grid.edit(cell); - matchedNodes = query("input", cell.element); + function checkAndMoveOn(expectedMatchCount, message){ + matchedNodes = query("input", cell.element); + assert.strictEqual(expectedMatchCount, matchedNodes.length, message); - if (canEdit(cell.row.data)) { - assert.strictEqual(1, matchedNodes.length, - "Cell with canEdit=>true should have an editor element"); + rowIndex++; + if(rowIndex < rowCount){ + testRow(rowIndex); + }else{ + dfd.resolve(); + } } - else { - assert.strictEqual(0, matchedNodes.length, - "Cell with canEdit=>false should not have an editor element"); + + cell = grid.cell(rowIndex, columnId); + editPromise = grid.edit(cell); + if(editPromise){ + editPromise.then(dfd.rejectOnError(function(){ + checkAndMoveOn(1, "Cell with canEdit=>true should have an editor element"); + })); + }else{ + checkAndMoveOn(0, "Cell with canEdit=>false should not have an editor element"); } } + testRow(0); + + return dfd; + } + + test.test("canEdit always on editor: suppress on false", function(){ + return testCanEdit(this.async(), "name"); }); + test.test("canEdit edit-on click editor: suppress on false", function(){ + return testCanEdit(this.async(), "description"); + }); test.test("destroy editor widgets: native", function () { var matchedNodes; @@ -208,7 +216,8 @@ define([ matchedNodes = query("input"); assert.strictEqual(testOrderedData.length, matchedNodes.length, - "There should be " + testOrderedData.length + " input elements for the grid's editors"); + "There should be " + testOrderedData.length + " input elements for the grid's editors and there were " + + matchedNodes.length); grid.destroy(); @@ -250,25 +259,11 @@ define([ "After grid is destroyed there should be 0 widgets on the page"); }); - - // Goal: test that when "grid.edit(cell)" is called the cell gets an editor with focus - // - // Observed behavior: - // In a cell without an always-on editor, if you call "grid.edit(cell)" - // repeatedly, the previously edited cell loses its content (not just its editor). - // document.activeElement.blur() between calls of "grid.edit" does not - // seem to work in this automated test, though it is of no consequence - // since this test is simply testing the editor's presence, not its after-effects. - // - // grid.edit: - // In a cell with an always-on editor, the editor's "focus" event is fired and - // "document.activeElement" is set to the editor. - // In a cell with a click-to-activate editor, no "focus" event is fired and - // "document.activeElement" is the body. - test.test("editor focus", function () { - var rowIndex, + test.test("editor focus with always on editor", function () { + var rowCount, cell, - cellEditor; + cellEditor, + dfd = this.async(); grid = new OnDemandGrid({ columns: { @@ -287,37 +282,82 @@ define([ document.body.appendChild(grid.domNode); grid.startup(); grid.renderArray(testOrderedData); + rowCount = testOrderedData.length; - for (rowIndex = 0; rowIndex < testOrderedData.length; rowIndex++) { - // Calling 'grid.edit()' on different cells consecutively results in the last-edited - // cell losing its content. It seems the blur process is important, so try to trigger that: - document.activeElement.blur(); - + function testRow(rowIndex){ // Test calling 'grid.edit()' in an always-on cell cell = grid.cell(rowIndex, "name"); - grid.edit(cell); - + grid.edit(cell).then(dfd.rejectOnError(function(node){ cellEditor = query("input", cell.element)[0]; + assert.strictEqual(cellEditor, node, + "edit method's promise should return the active editor"); assert.strictEqual(cellEditor, document.activeElement, "Editing a cell should make the cell's editor active"); + rowIndex++; + if(rowIndex < rowCount){ + testRow(rowIndex); + }else{ + dfd.resolve(); + } + })); + } + testRow(0); - document.activeElement.blur(); + return dfd; + }); + + test.test("editor focus and show event with edit-on click editor", function () { + var rowCount, + cell, + cellEditor, + dfd = this.async(); + grid = new OnDemandGrid({ + columns: { + order: "step", + name: editor({ + label: "Name", + editor: "text" + }), + description: editor({ + label: "Description", + editor: "text", + editOn: "click" + }) + } + }); + document.body.appendChild(grid.domNode); + grid.startup(); + grid.renderArray(testOrderedData); + rowCount = testOrderedData.length; + + function testRow(rowIndex){ + // Test calling 'grid.edit()' in an always-on cell cell = grid.cell(rowIndex, "description"); // Respond to the "dgrid-editor-show" event to ensure the // correct cell has an editor. This event actually fires // synchronously, so we don't need to use this.async. - on.once(grid.domNode, "dgrid-editor-show", function (event) { - // document.activeElement is the body for some reason. - // So at least check to ensure that the cell we called edit on - // is the same as the cell passed to the "dgrid-editor-show" event. + on.once(grid.domNode, "dgrid-editor-show", dfd.rejectOnError(function(event){ assert.strictEqual(cell.element, event.cell.element, "The activated cell should be being edited" ); - }); - - grid.edit(cell); + })); + // Don't move on to the next row until, the editor has received focus and the show event has fired. + grid.edit(cell).then(dfd.rejectOnError(function(){ + cellEditor = query("input", cell.element)[0]; + assert.strictEqual(cellEditor, document.activeElement, + "Editing a cell should make the cell's editor active"); + rowIndex++; + if(rowIndex < rowCount){ + testRow(rowIndex); + }else{ + dfd.resolve(); + } + })); } + testRow(0); + + return dfd; }); }); });