Discussion: Should every cell always be contenteditable?

[timwis]

At the moment, every <th> and <td> has contenteditable set to true unless the column is autoincrementing. When the user begins typing, presses paste, or hits enter, the sheet enters editing mode, which adds a drop shadow to the selected cell and tells the onblur listener to save changes. When the cell is blurred (or the user presses escape), editing mode is deactivated.

An alternative is to only make <th>s and <td>s contenteditable during edit mode. In theory, this should be more performant(?) because there's only a single contenteditable cell at a time. Also, we don't have to do the cursor magic when navigating between cells to ensure the entire contents are highlighted.

Here's how it can be accomplished:

diff --git a/src/components/grid.vue b/src/components/grid.vue
index 68cc95b..2132fc0 100644
--- a/src/components/grid.vue
+++ b/src/components/grid.vue
@@ -8,7 +8,6 @@
       @dblclick="onDblClickCell"
       @focus.capture="onFocus"
       @blur.capture="onBlur"
-      @input="onInput"
       @keydown.enter.capture.prevent="onPressEnter"
       @keydown.esc.capture.prevent="onPressEscape"
       @keydown.up.capture="onPressArrowKeys('up', $event)"
@@ -28,11 +27,11 @@
         </tr>
       </thead>
       <tbody @contextmenu.prevent="onRowContextMenu">
        <tr v-for="(row, rowIndex) in rows">
           <td
             v-for="(column, columnIndex) in columns"
             tabindex="0"
-            :contenteditable="(hasKeys && column.editable) ? true : false"
+            :contenteditable="(editing) ? true : false"
             :data-row-index="rowIndex"
             :data-column-index="columnIndex"
             v-text="row[column.name]"></td>
@@ -99,6 +98,14 @@ module.exports = {
       editing: (state) => state.ui.editing
     })
   },
+  mounted () {
+    window.addEventListener('copy', this.onCopy)
+    window.addEventListener('paste', this.onPaste)
+  },
+  destroyed () {
+    window.removeEventListener('copy', this.onCopy)
+    window.removeEventListener('paste', this.onPaste)
+  },
   methods: {
     ...mapMutations([
       'setEditing',
@@ -135,12 +142,27 @@ module.exports = {
         }
       }
     },
-    onInput (evt) {
-      if (!this.editing) this.setEditing()
+    onKeyPress (evt) {
+      if (this.editing) return
+
+      const el = evt.target
+      const { columnIndex } = getElIndexes(el)
+      const isEditable = this.columns[columnIndex].editable
+      if (isEditable) this.setEditing()
+    },
+    onCopy (evt) {
+      const el = document.activeElement
+      evt.clipboardData.setData('text/plain', el.textContent)
+      evt.preventDefault()
+    },
+    onPaste (evt) {
+      const el = document.activeElement
+      this.onKeyPress({ target: el })
     },
     onPressEnter (evt) {
       const el = evt.target
-      const isEditable = (el.getAttribute('contenteditable') === 'true') // no jsdom support for isContentEditable
+      const { columnIndex } = getElIndexes(el)
+      const isEditable = this.columns[columnIndex].editable
       if (this.editing) {
         // navigating down triggers blur, which triggers setNotEditing
         this.navigate('down', evt.target)
@@ -165,7 +187,8 @@ module.exports = {
     },
     onDblClickCell (evt) {
       const el = evt.target
-      const isEditable = el.isContentEditable
+      const { columnIndex } = getElIndexes(el)
+      const isEditable = this.columns[columnIndex].editable
       if (isEditable) {
         this.setEditing()
         el.focus()

For some reason, entering editing mode is slower on large tables with this route (which is quite peculiar). Copy and paste works well, and sets us up to handle more complicated copies and pastes (like handling multiple cells). But the former approach has the advantage of copy and past just working, and it actually supports undo/redo -- multiple of them!!

So I think the former approach wins, even though it's a bit odder. Really just leaving this discussion here for posterity.