Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated editor tests to wait for edit() promise to resolve #903

Closed
wants to merge 3 commits into from
Closed

Updated editor tests to wait for edit() promise to resolve #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db44e5d
Fixed the editor unit tests that were causing errors (but not test fa…
edhager Apr 21, 2014
ab4077c
I goofed before and added code that was not compatible with Dojo 1.7.…
edhager Apr 23, 2014
d54d05d
Blur the shared editor only if it is visible.
edhager Apr 23, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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"){
Expand Down Expand Up @@ -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){
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -363,14 +360,15 @@ 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; }

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){
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
Expand Down Expand Up @@ -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);
});
}

Expand Down
164 changes: 102 additions & 62 deletions test/intern/plugins/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)");
Expand All @@ -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",
Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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: {
Expand All @@ -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;
});
});
});