Make more changes related to submission actions

The main change here is that the .updated highlight will be removed
after 6s.

src/components/submission/list.vue

@@ -31,10 +31,9 @@ except according to the terms contained in the LICENSE file.
           @decrypt="showDecrypt"/>
       </div>
       <submission-table v-show="submissions != null && submissions.length !== 0"
-        :project-id="projectId" :xml-form-id="xmlFormId" :draft="draft"
-        :submissions="submissions" :fields="selectedFields"
-        :original-count="originalCount" :updated="updated"
-        @review="showReview"/>
+        ref="table" :project-id="projectId" :xml-form-id="xmlFormId"
+        :draft="draft" :submissions="submissions" :fields="selectedFields"
+        :original-count="originalCount" @review="showReview"/>
       <p v-show="submissions != null && submissions.length === 0"
         class="empty-table-message">
         {{ odataFilter == null ? $t('emptyTable') : $t('noMatching') }}
@@ -120,9 +119,6 @@ export default {
       // equal submissions.length unless a submission has been created since the
       // initial fetch or last refresh.
       skip: 0,
-      // The index of the submission whose review state was updated most
-      // recently
-      updated: null,
       decrypt: {
         state: false,
         formAction: null
@@ -221,15 +217,13 @@ export default {
       this.instanceIds.clear();
       this.originalCount = null;
       this.skip = 0;
-      this.updated = null;
     },
     replaceSubmissions() {
       this.submissions = this.odataChunk.value;
       this.instanceIds.clear();
       for (const submission of this.submissions)
         this.instanceIds.add(submission.__id);
       this.originalCount = this.odataChunk['@odata.count'];
-      this.updated = null;
     },
     pushSubmissions() {
       const lastSubmissionDate = last(this.submissions).__system.submissionDate;
@@ -336,9 +330,7 @@ export default {
           ...submission,
           __system: { ...submission.__system, reviewState }
         });
-        this.updated = index;
-      } else {
-        this.updated = null;
+        this.$refs.table.afterReview(index);
       }
     }
   }

src/components/submission/metadata-row.vue

@@ -123,7 +123,12 @@ export default {
 @import '../../assets/scss/mixins';
 
 .submission-metadata-row {
-  &.updated { background-color: #faf1cd; }
+  transition: background-color 0.6s 6s;
+
+  &.updated {
+    background-color: #faf1cd;
+    transition: none;
+  }
 
   .row-number {
     color: #999;

src/components/submission/table.vue

@@ -21,7 +21,7 @@ except according to the terms contained in the LICENSE file.
         </tr>
       </thead>
       <tbody ref="metadataBody"
-        :class="`submission-table-actions-trigger-${actions.trigger}`"
+        :class="`submission-table-actions-trigger-${actionsTrigger}`"
         @mousemove="setActionsTrigger('hover')"
         @focusin="setActionsTrigger('focus')" @click="review">
         <template v-if="submissions != null">
@@ -84,32 +84,29 @@ export default {
     draft: Boolean,
     submissions: Array,
     fields: Array,
-    originalCount: Number,
-    updated: Number
+    originalCount: Number
   },
   data() {
     return {
-      actions: {
-        /*
-        Actions are shown for a row if the cursor is over the row or if one of
-        the actions is focused. However, it is possible for the cursor to be
-        over one row while an action is focused in a different row. In that
-        case, we show the actions for one of the two rows depending on the type
-        of the most recent event.
-
-        I tried other approaches before landing on this one. However, sequences
-        of events like the following were a challenge:
-
-          - Click the More button for a row.
-          - Next, press tab to focus the Review button in the next row.
-          - Actions are shown for the next row and are no longer shown beneath
-            the cursor. However, that will trigger a mouseover event, which
-            depending on the approach may cause actions to be shown beneath the
-            cursor again.
-        */
-        trigger: 'hover',
-        dataHover: null
-      }
+      /*
+      Actions are shown for a row if the cursor is over the row or if one of the
+      actions is focused. However, it is possible for the cursor to be over one
+      row while an action is focused in a different row. In that case, we show
+      the actions for one of the two rows depending on the type of the most
+      recent event.
+
+      I tried other approaches before landing on this one. However, sequences of
+      events like the following were a challenge:
+
+        - Click the More button for a row.
+        - Next, press tab to focus the Review button in the next row.
+        - Actions are shown for the next row and are no longer shown beneath the
+          cursor. However, that will trigger a mouseover event, which depending
+          on the approach may cause actions to be shown beneath the cursor
+          again.
+      */
+      actionsTrigger: 'hover',
+      dataHover: null
     };
   },
   computed: {
@@ -121,47 +118,61 @@ export default {
     }
   },
   watch: {
-    submissions: 'removeHoverClass',
-    updated(index) {
-      const { metadataBody } = this.$refs;
-      const oldRow = metadataBody.querySelector('.updated');
-      if (oldRow != null) oldRow.classList.remove('updated');
-
-      if (index != null) {
-        const newRow = metadataBody.querySelector(`tr:nth-child(${index + 1})`);
-        // The SubmissionMetadataRow element does not have a class binding, so I
-        // think we can add this class without Vue removing it.
-        newRow.classList.add('updated');
-      }
-    }
+    /*
+    We remove the data-hover class after the submissions are refreshed, with the
+    following cases in mind:
+
+      - There may be fewer submissions after the refresh than before. In that
+        case, it is possible that this.submissions.length <= this.dataHover.
+      - A submission may be in a different row after the refresh. For example,
+        if the user hovers over the first row, and after the refresh, that
+        submission is in the second row, then the second row will incorrectly
+        have the data-hover class.
+
+    In some cases, it would be ideal not to remove the class or to add the class
+    to the row for a different submission. That logic is not in place right now.
+    */
+    submissions: 'removeHoverClass'
   },
   methods: {
     setActionsTrigger(trigger) {
-      this.actions.trigger = trigger;
+      this.actionsTrigger = trigger;
     },
     toggleHoverClass(event) {
       const dataRow = event.target.closest('tr');
       const index = Number.parseInt(dataRow.dataset.index, 10);
-      if (index === this.actions.dataHover) return;
+      if (index === this.dataHover) return;
       const { metadataBody } = this.$refs;
-      if (this.actions.dataHover != null)
+      if (this.dataHover != null)
         metadataBody.querySelector('.data-hover').classList.remove('data-hover');
       const metadataRow = metadataBody.querySelector(`tr:nth-child(${index + 1})`);
+      // The SubmissionMetadataRow element does not have a class binding, so I
+      // think we can add this class without Vue removing it.
       metadataRow.classList.add('data-hover');
-      this.actions.dataHover = index;
+      this.dataHover = index;
     },
     removeHoverClass() {
-      if (this.actions.dataHover != null) {
+      if (this.dataHover != null) {
         const tr = this.$refs.metadataBody.querySelector('.data-hover');
         tr.classList.remove('data-hover');
-        this.actions.dataHover = null;
+        this.dataHover = null;
       }
     },
     review(event) {
       if (!this.canUpdate) return;
       const tr = event.target.closest('tr');
       if (tr.querySelector('.review-button').contains(event.target))
         this.$emit('review', this.submissions[tr.dataset.index]);
+    },
+    // Using a method instead of a prop in case the same submission is updated
+    // twice in a row.
+    afterReview(index) {
+      const { metadataBody } = this.$refs;
+      const tr = metadataBody.querySelector(`tr:nth-child(${index + 1})`);
+      tr.classList.add('updated');
+      this.$nextTick(() => {
+        tr.classList.remove('updated');
+      });
     }
   }
 };

test/components/submission/metadata-row.spec.js

@@ -1,5 +1,8 @@
+import sinon from 'sinon';
+
 import DateTime from '../../../src/components/date-time.vue';
 import SubmissionMetadataRow from '../../../src/components/submission/metadata-row.vue';
+import SubmissionTable from '../../../src/components/submission/table.vue';
 import SubmissionUpdateReviewState from '../../../src/components/submission/update-review-state.vue';
 
 import testData from '../../data';
@@ -139,9 +142,10 @@ describe('SubmissionMetadataRow', () => {
 
     describe('after a successful response', () => {
       const submit = () => {
-        testData.extendedSubmissions
-          .createPast(1, { instanceId: 'foo', reviewState: null })
-          .createPast(1, { instanceId: 'bar', reviewState: null });
+        testData.extendedSubmissions.createPast(1, {
+          instanceId: 'foo',
+          reviewState: null
+        });
         return loadSubmissionList()
           .complete()
           .request(async (component) => {
@@ -174,52 +178,20 @@ describe('SubmissionMetadataRow', () => {
         const submission = component.data().submissions[0];
         submission.__system.reviewState.should.equal('hasIssues');
         // Check that other properties were copied correctly.
-        submission.__id.should.equal('bar');
+        submission.__id.should.equal('foo');
         submission.__system.submitterId.should.equal('1');
       });
 
-      describe('highlight', () => {
-        it('highlights the row', async () => {
-          const component = await submit();
-          const row = component.first(SubmissionMetadataRow);
-          row.hasClass('updated').should.be.true();
-        });
-
-        it('toggles the highlight if another row is updated', () =>
-          submit()
-            .complete()
-            .request(async (component) => {
-              const row = component.find(SubmissionMetadataRow)[1];
-              await trigger.click(row, '.review-button');
-              return trigger.submit(component, '#submission-update-review-state form', [
-                ['input[value="hasIssues"]', true]
-              ]);
-            })
-            .respondWithData(() => {
-              testData.extendedSubmissions.update(0, {
-                reviewState: 'hasIssues'
-              });
-              return testData.standardSubmissions.first();
-            })
-            .afterResponse(component => {
-              const rows = component.find(SubmissionMetadataRow);
-              rows[0].hasClass('updated').should.be.false();
-              rows[1].hasClass('updated').should.be.true();
-            }));
-
-        it('removes the highlight after the submissions are refreshed', () =>
-          submit()
-            .complete()
-            .request(trigger.click('#submission-list-refresh-button'))
-            .beforeEachResponse(component => {
-              const row = component.first(SubmissionMetadataRow);
-              row.hasClass('updated').should.be.true();
-            })
-            .respondWithData(testData.submissionOData)
-            .afterResponse(component => {
-              const row = component.first(SubmissionMetadataRow);
-              row.hasClass('updated').should.be.false();
-            }));
+      it('calls SubmissionTable.methods.afterReview()', () => {
+        const afterReview = sinon.fake();
+        return submit()
+          .beforeAnyResponse(component => {
+            const table = component.first(SubmissionTable);
+            sinon.replace(table.vm, 'afterReview', afterReview);
+          })
+          .afterResponse(() => {
+            afterReview.called.should.be.true();
+          });
       });
     });
   });