Browse Source

fix(webapp): highlight table rows by setting state

Manipulation of the DOM is the exclusive job of vue.js. Modification
of the DOM directly yields to unexpected outcome, like in this case
where row highlighting could be messed up by changing table sort
order (the highlight would remain in place even if the row moves
away).

I think it's acceptable for now to keep the 'successFade' DOM
manipulation, as it is very short term.
Nils Wisiol 4 years ago
parent
commit
907da7d5b4
1 changed files with 20 additions and 10 deletions
  1. 20 10
      webapp/src/views/CrudList.vue

+ 20 - 10
webapp/src/views/CrudList.vue

@@ -20,7 +20,7 @@
           <!-- The Actual Table -->
           <!-- The Actual Table -->
           <v-data-table
           <v-data-table
                   :headers="headers"
                   :headers="headers"
-                  :item-class="(item) => itemIsReadOnly(item) ? 'text--disabled grey lighten-4' : ''"
+                  :item-class="itemClass"
                   :items="rows"
                   :items="rows"
                   :search="search"
                   :search="search"
                   :custom-filter="filterSearchableCols"
                   :custom-filter="filterSearchableCols"
@@ -206,7 +206,7 @@
                 v-model="itemFieldProps.item[column.value]"
                 v-model="itemFieldProps.item[column.value]"
                 v-bind="column.fieldProps ? column.fieldProps(itemFieldProps.item) : {}"
                 v-bind="column.fieldProps ? column.fieldProps(itemFieldProps.item) : {}"
                 @keyup="keyupHandler"
                 @keyup="keyupHandler"
-                @dirty="dirtyHandler"
+                @dirty="dirty.add(itemFieldProps.item); dirtyError.delete(itemFieldProps.item);"
               />
               />
             </template>
             </template>
             <template v-slot:[`item.actions`]="itemFieldProps">
             <template v-slot:[`item.actions`]="itemFieldProps">
@@ -394,6 +394,8 @@ export default {
     search: '',
     search: '',
     rows: [],
     rows: [],
     valid: false,
     valid: false,
+    dirty: new Set(),
+    dirtyError: new Set(),
     /* to be overwritten */
     /* to be overwritten */
     // features
     // features
     createable: true,
     createable: true,
@@ -430,7 +432,6 @@ export default {
     itemIsReadOnly: () => false,
     itemIsReadOnly: () => false,
     postcreate: this.close,
     postcreate: this.close,
     precreate: () => undefined,
     precreate: () => undefined,
-    dirtyHandler: (e) => e.target.closest('tr').classList.add('orange', 'lighten-5'),
     keyupHandler: (e) => {
     keyupHandler: (e) => {
       // Intercept Enter key
       // Intercept Enter key
       if (e.keyCode === 13) {
       if (e.keyCode === 13) {
@@ -474,6 +475,18 @@ export default {
     this.createDialogItem = Object.assign({}, this.itemDefaults());
     this.createDialogItem = Object.assign({}, this.itemDefaults());
   },
   },
   methods: {
   methods: {
+    itemClass(item) {
+      if (this.itemIsReadOnly(item)) {
+        return 'grey text--disabled grey lighten-4';
+      }
+      if (this.dirtyError.has(item)) {
+        return 'red lighten-5';
+      }
+      if (this.dirty.has(item)) {
+        return 'orange lighten-5';
+      }
+      return '';
+    },
     filterWriteableColumns(callback) {
     filterWriteableColumns(callback) {
       const columns = filter(this.columns, c => !c.readonly || c.writeOnCreate);
       const columns = filter(this.columns, c => !c.readonly || c.writeOnCreate);
       return filter(columns, callback);
       return filter(columns, callback);
@@ -536,6 +549,7 @@ export default {
         // edit item
         // edit item
         let tr;
         let tr;
         if (event) {
         if (event) {
+          // TODO do not temper with the DOM directly -- bad things will happen (see commit msg)
           tr = event.target.closest('tr');
           tr = event.target.closest('tr');
           tr.addEventListener("animationend", () => tr.classList.remove('successFade'), true);
           tr.addEventListener("animationend", () => tr.classList.remove('successFade'), true);
           tr.classList.add('successFade');
           tr.classList.add('successFade');
@@ -549,15 +563,11 @@ export default {
                 .patch(url, item)
                 .patch(url, item)
                 .then(r => {
                 .then(r => {
                   self.rows[self.rows.indexOf(item)] = r.data;
                   self.rows[self.rows.indexOf(item)] = r.data;
-                  if (event) {
-                    tr.classList.remove('orange', 'red', 'lighten-5');
-                  }
+                  self.dirty.delete(item);
+                  self.dirtyError.delete(item);
                 })
                 })
                 .catch(function (error) {
                 .catch(function (error) {
-                  if (event) {
-                    tr.classList.remove('orange', 'red');
-                    tr.classList.add('red', 'lighten-5');
-                  }
+                  self.dirtyError.add(item);
                   throw error;
                   throw error;
                 })
                 })
         );
         );