The new listing is not editable. It would be a nice feature.
Created attachment 27855 [details] Make DOMStorage view editable.
Comment on attachment 27855 [details] Make DOMStorage view editable. > + get statusBarItems() > + { > + return [this.deleteButton]; > + }, The delete button should be on the view side of the status bar, like we do in the ProfilesPanel/ProfilesView. A delete button here would imply deleting a local/session storage object not a row. r- for this > + this.visibleView._dataGrid._deleteRow(); Functions that are "public" like _deleteRow() should not have an underscore. Also deleteSelectedRow() would be best, since the row index isn't passed. > + var element = event.target.enclosingNodeOrSelfWithNodeName("td"); Why do you need to look for a element? Can't you get everything from dataGridNodeFromEvent? > + } > + else { This should be: } else { on the same line. > + if (node && domStorage) { > + domStorage.removeItem(node.data[0]); > + } No need for braces here.
(In reply to comment #2) > (From update of attachment 27855 [details] [review]) > > > + get statusBarItems() > > + { > > + return [this.deleteButton]; > > + }, > > The delete button should be on the view side of the status bar, like we do in > the ProfilesPanel/ProfilesView. A delete button here would imply deleting a > local/session storage object not a row. r- for this > > > + this.visibleView._dataGrid._deleteRow(); > > Functions that are "public" like _deleteRow() should not have an underscore. > Also deleteSelectedRow() would be best, since the row index isn't passed. > > > + var element = event.target.enclosingNodeOrSelfWithNodeName("td"); > > Why do you need to look for a element? Can't you get everything from > dataGridNodeFromEvent? > > > + } > > + else { > > This should be: } else { on the same line. > > > > + if (node && domStorage) { > > + domStorage.removeItem(node.data[0]); > > + } > > No need for braces here. > Thank you for the review. I will address your comments in my next patch. Regarding > + var element = event.target.enclosingNodeOrSelfWithNodeName("td"); I did that because I need to know not only the node, but also which of the columns was double clicked (is it key or is it value). Could you suggest a better way to find which column was clicked? thanks!
Created attachment 28017 [details] Addressing comment #2. This patch is addressing comment #2, with the exception that I am still using var element = event.target.enclosingNodeOrSelfWithNodeName("td"); I am not sure how else to find out if a key was edited or a value was edited.
Comment on attachment 28017 [details] Addressing comment #2. Using enclosingNodeOrSelfWithNodeName("td") is fine, now that I understand it. Nice work!
Thank you for the review. Now I need help committing this patch :-)
Landed in r41288.