From 4f57bc1150542a2fe91565cf61858f3bc77bdcb1 Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Thu, 24 May 2012 13:05:03 +1000 Subject: [PATCH 1/8] Use column's grid member if this is not column object (relevant when editor wraps a tree) --- tree.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tree.js b/tree.js index d74c73975..8a1636ddb 100644 --- a/tree.js +++ b/tree.js @@ -14,7 +14,9 @@ return function(column){ // Renders a cell that can be expanded, creating more rows var level = Number(options.query.level) + 1; level = isNaN(level) ? 0 : level; - var grid = this.grid; + // 'this' might not be the column object, if this is called from editor's renderCell function, so + // in which case use column's grid member + var grid = this.grid || column.grid; var transitionEventSupported; var mayHaveChildren = !grid.store.mayHaveChildren || grid.store.mayHaveChildren(object); // create the expando From 5dfe2fd6f32c7bf898e6145457cdf795c1dd931a Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Thu, 24 May 2012 13:23:57 +1000 Subject: [PATCH 2/8] options param isn't defined when tree wrapped in editor --- tree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree.js b/tree.js index 8a1636ddb..d9aa066aa 100644 --- a/tree.js +++ b/tree.js @@ -12,7 +12,7 @@ return function(column){ column.renderCell = function(object, value, td, options){ // summary: // Renders a cell that can be expanded, creating more rows - var level = Number(options.query.level) + 1; + var level = options ? Number(options.query.level) + 1 : 0; level = isNaN(level) ? 0 : level; // 'this' might not be the column object, if this is called from editor's renderCell function, so // in which case use column's grid member From aab2be3ace1c4c36a880ba4fdccb4fdbe19f69e9 Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Fri, 25 May 2012 09:41:53 +1000 Subject: [PATCH 3/8] If a tree is wrapped in an editor, the expand tree event is now successfully handled independently of the activate editor event --- editor.js | 37 ++++++++++++++++++++++++------------- tree.js | 4 ++-- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/editor.js b/editor.js index dcd8c9265..b5e76fdec 100644 --- a/editor.js +++ b/editor.js @@ -7,8 +7,9 @@ define([ "dojo/has", "./Grid", "put-selector/put", - "dojo/_base/sniff" -], function(kernel, lang, Deferred, on, aspect, has, Grid, put){ + "dojo/query", + "dojo/_base/sniff", +], function(kernel, lang, Deferred, on, aspect, has, Grid, put, query){ var activeCell, activeValue; // tracks cell currently being edited, and its value @@ -351,17 +352,27 @@ return function(column, editor, editOn){ column.editorInstance.destroyRecursive(); }); } - - // 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) - - // 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(){ grid.edit(this); }); - - // initially render content in non-edit mode - return originalRenderCell(object, value, cell, options); + + // initially render content in non-edit mode + var nonEditNode = originalRenderCell(object, value, cell, options); + + if (!column._evtHandlerAdded) { + var colSelector = ".dgrid-content .dgrid-column-" + column.id; + // Does the editor wrap a tree? If so, get the content node + var treeExpandoClass = ".dgrid-expando-text"; + var nl = query(treeExpandoClass, cell); + if (nl.length > 0) { + // TODO: This doesn't work with event dgrid-cellfocusin so event + // click needs to be used when you wrap a tree within an editor + colSelector += " " + treeExpandoClass; + } + var eventType = colSelector + ":" + editOn; + grid.on(eventType, function(evt) { + grid.edit(this); + }); + column._evtHandlerAdded = true; + }; + return nonEditNode; } : function(object, value, cell, options){ // always-on: create editor immediately upon rendering each cell var grid = column.grid, diff --git a/tree.js b/tree.js index d9aa066aa..db7397ccd 100644 --- a/tree.js +++ b/tree.js @@ -14,8 +14,8 @@ return function(column){ // Renders a cell that can be expanded, creating more rows var level = options ? Number(options.query.level) + 1 : 0; level = isNaN(level) ? 0 : level; - // 'this' might not be the column object, if this is called from editor's renderCell function, so - // in which case use column's grid member + // 'this' might not be the column object, if this is called from editor's renderCell function, so + // in which case use column's grid member var grid = this.grid || column.grid; var transitionEventSupported; var mayHaveChildren = !grid.store.mayHaveChildren || grid.store.mayHaveChildren(object); From 7773f77f5fcaa1e2757efaed4952737cf864eb63 Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Fri, 25 May 2012 14:00:49 +1000 Subject: [PATCH 4/8] When editing done indented tree elements are drawn correctly. If the editor wraps a tree, and you edit a child node, then when editing is complete it renders with the correct level of indentation. To do this I had to make sure that the options object used to render original child is used at the end of the edit. --- List.js | 11 +++++++++++ editor.js | 51 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/List.js b/List.js index 1f8df30f9..56cba940f 100644 --- a/List.js +++ b/List.js @@ -462,6 +462,17 @@ function(arrayUtil, kernel, declare, listen, aspect, has, miscUtil, TouchScroll, renderHeader: function(){ // no-op in a plain list }, + _rowIdForObject: function(object) { + // summary: + // Returns the row dom id for the row that represents passed object + // description: + // Useful to code which needs row ID before row has finished + // being created (see editor for example). + // + // A side-effect of calling this function is that this._autoId + // will be incremented if this List doesn't use a store + return this.id + "-row-" + ((this.store && this.store.getIdentity) ? this.store.getIdentity(object) : this._autoId++); + }, insertRow: function(object, parent, beforeNode, i, options){ // summary: // Renders a single row in the grid diff --git a/editor.js b/editor.js index b5e76fdec..50f729bbb 100644 --- a/editor.js +++ b/editor.js @@ -193,7 +193,9 @@ function createSharedEditor(column, originalRenderCell){ // pass new value to original renderCell implementation for this cell Grid.appendIfNode(parentNode, originalRenderCell( - column.grid.row(parentNode).data, activeValue, parentNode)); + column.grid.row(parentNode).data, activeValue, parentNode, + cmp._optionsForCell)); + delete cmp._optionsForCell; // reset state now that editor is deactivated activeCell = activeValue = null; @@ -255,12 +257,14 @@ function showEditor(cmp, column, cell, value){ if(activeCell){ activeValue = value; } } -function edit(cell) { +function edit(cell, options) { // summary: // Method to be mixed into grid instances, which will show/focus the // editor for a given grid cell. Also used by renderCell. // cell: Object // Cell (or something resolvable by grid.cell) to activate editor on. + // options: Object (optional) + // options used to render original cell // returns: // If the cell is editable, returns a promise resolving to the editor // input/widget when the cell editor is focused. @@ -281,7 +285,11 @@ function edit(cell) { dirty = this.dirty && this.dirty[row.id]; value = (dirty && field in dirty) ? dirty[field] : column.get ? column.get(row.data) : row.data[field]; - + + // Smuggle in options so that onblur() above (which is within createSharedEditor()) + // can use the correct options when calling originalRenderCell(). This is needed + // if editor wraps a tree plugin. + cmp._optionsForCell = options; showEditor(column.editorInstance, column, cellElement, value); // focus / blur-handler-resume logic is surrounded in a setTimeout @@ -335,6 +343,10 @@ return function(column, editor, editOn){ column.renderCell = editOn ? function(object, value, cell, options){ var grid = column.grid; + if (!(grid.store && grid.store.getIdentity)) { + throw new Error("Editor currently only works for store-backed grids"); + } + // On first run, create one shared widget/input which will be swapped into // the active cell. if(!column.editorInstance){ @@ -355,23 +367,22 @@ return function(column, editor, editOn){ // initially render content in non-edit mode var nonEditNode = originalRenderCell(object, value, cell, options); - - if (!column._evtHandlerAdded) { - var colSelector = ".dgrid-content .dgrid-column-" + column.id; - // Does the editor wrap a tree? If so, get the content node - var treeExpandoClass = ".dgrid-expando-text"; - var nl = query(treeExpandoClass, cell); - if (nl.length > 0) { - // TODO: This doesn't work with event dgrid-cellfocusin so event - // click needs to be used when you wrap a tree within an editor - colSelector += " " + treeExpandoClass; - } - var eventType = colSelector + ":" + editOn; - grid.on(eventType, function(evt) { - grid.edit(this); - }); - column._evtHandlerAdded = true; - }; + + // Use grid.on to target exact element for event handling + var rowSel = ".dgrid-row#" + grid._rowIdForObject(object); + var colSelector = ".dgrid-content " + rowSel + " .dgrid-column-" + column.id; + // Does the editor wrap a tree? If so, get the content node + var treeExpandoClass = ".dgrid-expando-text"; + var nl = query(treeExpandoClass, cell); + if (nl.length > 0) { + // TODO: This doesn't work with event dgrid-cellfocusin so event + // click needs to be used when you wrap a tree within an editor + colSelector += " " + treeExpandoClass; + } + var eventType = colSelector + ":" + editOn; + grid.on(eventType, function(evt) { + grid.edit(this, options); + }); return nonEditNode; } : function(object, value, cell, options){ // always-on: create editor immediately upon rendering each cell From ca6aabe09aed5354f05462a5c33a3439642a38ef Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Fri, 25 May 2012 14:55:39 +1000 Subject: [PATCH 5/8] Prevent space key during edit from expanding tree node --- tree.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tree.js b/tree.js index db7397ccd..fd5d2a882 100644 --- a/tree.js +++ b/tree.js @@ -70,6 +70,12 @@ return function(column){ target = target.element || target; // if a row object was passed in, get the element first target = target.className.indexOf("dgrid-expando-icon") > -1 ? target : querySelector(".dgrid-expando-icon", target)[0]; + if (!target) { + // We get here if tree is wrapped in an editor and the editor is active, in which + // case the user shouldn't be able to expand tree and this call is a result of + // user pressing space key, so ignore it + return; + } if(target.mayHaveChildren){ // on click we toggle expanding and collapsing var expanded = target.expanded = expand === undefined ? !target.expanded : expand; From 995eff39385bff318ac9fdaec254b9683aa0a7ef Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Tue, 29 May 2012 09:50:39 +1000 Subject: [PATCH 6/8] Added optional column property 'onEdit' which is called by editor if an editor widget is invoked. Both the row data object and the widget are passed --- editor.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/editor.js b/editor.js index 50f729bbb..308621ae8 100644 --- a/editor.js +++ b/editor.js @@ -249,6 +249,10 @@ function showEditor(cmp, column, cell, value){ // (Clear flag on a timeout to wait for delayed onChange to fire first) cmp._dgridIgnoreChange = true; cmp.set("value", value); + if (column.onEdit) { + var obj = grid.cell(cell).row.data; + column.onEdit(obj, cmp); + } setTimeout(function(){ cmp._dgridIgnoreChange = false; }, 0); } // track previous value for short-circuiting or in case we need to revert From 802b32a46fe1dffe82017c963b380a594b62c8b1 Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Tue, 29 May 2012 10:42:58 +1000 Subject: [PATCH 7/8] 'Always on' editors now check column.canEdit() --- editor.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/editor.js b/editor.js index 308621ae8..d4c445dc8 100644 --- a/editor.js +++ b/editor.js @@ -390,18 +390,16 @@ return function(column, editor, editOn){ return nonEditNode; } : function(object, value, cell, options){ // always-on: create editor immediately upon rendering each cell - var grid = column.grid, - cmp = createEditor(column); - + var grid = column.grid; if(!grid.edit){ grid.edit = edit; // add edit method on first render } - - showEditor(cmp, column, cell, value); - - // Maintain reference for later use. - cell[isWidget ? "widget" : "input"] = cmp; - + if (!column.canEdit || column.canEdit(object, value)) { + var cmp = createEditor(column); + showEditor(cmp, column, cell, value); + // Maintain reference for later use. + cell[isWidget ? "widget" : "input"] = cmp; + } if(isWidget){ if(!cleanupAdded){ cleanupAdded = true; From 590f72a6b4eaef92f91940eb02f86cb4d7428ec7 Mon Sep 17 00:00:00 2001 From: Yasir Assam Date: Tue, 5 Jun 2012 14:36:30 +1000 Subject: [PATCH 8/8] If the editor wraps a tree and the editor is active pressing the space key won't try to expand tree. Make sure 'this' points to column object when calling originalRenderCell from editor (so that tree can see correct version of this). --- editor.js | 4 ++-- tree.js | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/editor.js b/editor.js index d4c445dc8..fbac26f00 100644 --- a/editor.js +++ b/editor.js @@ -192,7 +192,7 @@ function createSharedEditor(column, originalRenderCell){ parentNode.removeChild(node); // pass new value to original renderCell implementation for this cell - Grid.appendIfNode(parentNode, originalRenderCell( + Grid.appendIfNode(parentNode, originalRenderCell.call(column, column.grid.row(parentNode).data, activeValue, parentNode, cmp._optionsForCell)); delete cmp._optionsForCell; @@ -370,7 +370,7 @@ return function(column, editor, editOn){ } // initially render content in non-edit mode - var nonEditNode = originalRenderCell(object, value, cell, options); + var nonEditNode = originalRenderCell.call(this, object, value, cell, options); // Use grid.on to target exact element for event handling var rowSel = ".dgrid-row#" + grid._rowIdForObject(object); diff --git a/tree.js b/tree.js index 5aed9f76d..f9ac379cf 100644 --- a/tree.js +++ b/tree.js @@ -93,7 +93,9 @@ return function(column){ target = target.element || target; // if a row object was passed in, get the element first target = target.className.indexOf("dgrid-expando-icon") > -1 ? target : querySelector(".dgrid-expando-icon", target)[0]; - if(target.mayHaveChildren){ + // Check if target is defined. If editor is wrapped in editor pressing space key whilst + // editing will trigger this event and target will be undefined + if(target && target.mayHaveChildren){ // on click we toggle expanding and collapsing var expanded = target.expanded = expand === undefined ? !target.expanded : expand; // update the expando display