From 81aec77059b8d42eb8894e2f425678ca024c56f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Mon, 30 May 2022 15:08:34 +0200
Subject: [PATCH] Resize the Tasks table when necessary

The `JobDetails` component now emits a `reshuffled` event whenever its
contents have changed, so that other components can respond to any changes
in available space. This event now triggers a resize of the tasks table
on the next DOM tick (so that the new sizes of the HTML elements are
available).

The `TasksTable` component also recomputes the table size when the task
status filters change (because that might have triggered a show/hide of
the filter bar). It also computes the available height slightly differently
so that it's all done relative to the tabulator element.

There is just one TODO left, which is a hard-coded offset of 12 that
should be obtained dynamically from somewhere -- no idea where it comes
from or why it's necessary.
---
 web/app/src/components/JobDetails.vue | 16 ++++++++---
 web/app/src/components/TasksTable.vue | 38 +++++++++++++++++++++------
 web/app/src/views/JobsView.vue        | 21 ++++++++++++++-
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/web/app/src/components/JobDetails.vue b/web/app/src/components/JobDetails.vue
index b2d7cc88..778d5a4b 100644
--- a/web/app/src/components/JobDetails.vue
+++ b/web/app/src/components/JobDetails.vue
@@ -59,6 +59,9 @@ export default {
   props: [
     "jobData", // Job data to show.
   ],
+  emits: [
+    "reshuffled", // Emitted when the size of this component may have changed. Used to resize other components in response.
+  ],
   data() {
     return {
       datetime: datetime, // So that the template can access it.
@@ -105,13 +108,13 @@ export default {
   methods: {
     _refreshJobSettings(newJobData) {
       if (objectEmpty(newJobData)) {
-        this.simpleSettings = null;
+        this._clearJobSettings();
         return;
       }
 
       // Only fetch the job type if it's different from what's already loaded.
       if (objectEmpty(this.jobType) || this.jobType.name != newJobData.type) {
-        this.simpleSettings = null; // They should only be shown when the type info is known.
+        this._clearJobSettings(); // They should only be shown when the type info is known.
 
         this.jobsApi.getJobType(newJobData.type)
           .then(this.onJobTypeLoaded)
@@ -136,14 +139,20 @@ export default {
       }
     },
 
+    _clearJobSettings() {
+      this.simpleSettings = null;
+      this.$emit('reshuffled');
+    },
+
     _setJobSettings(newJobSettings) {
       if (objectEmpty(newJobSettings)) {
-        this.simpleSettings = null;
+        this._clearJobSettings();
         return;
       }
 
       if (objectEmpty(this.jobTypeSettings)) {
         console.warn("empty job type settings");
+        this._clearJobSettings();
         return;
       }
 
@@ -167,6 +176,7 @@ export default {
       }
 
       this.simpleSettings = filtered;
+      this.$emit('reshuffled');
     }
   }
 };
diff --git a/web/app/src/components/TasksTable.vue b/web/app/src/components/TasksTable.vue
index f35ca920..ac5e75c5 100644
--- a/web/app/src/components/TasksTable.vue
+++ b/web/app/src/components/TasksTable.vue
@@ -10,6 +10,7 @@
   </div>
 </template>
 
+
 <script lang="js">
 import { TabulatorFull as Tabulator } from 'tabulator-tables';
 import * as datetime from "@/datetime";
@@ -84,10 +85,10 @@ export default {
     this.tabulator.on("rowClick", this.onRowClick);
     this.tabulator.on("tableBuilt", this._onTableBuilt);
 
-    window.addEventListener('resize', this._setTableHeight);
+    window.addEventListener('resize', this.recalcTableHeight);
   },
   unmounted() {
-    window.removeEventListener('resize', this._setTableHeight);
+    window.removeEventListener('resize', this.recalcTableHeight);
   },
   watch: {
     jobID() {
@@ -97,6 +98,12 @@ export default {
       this._reformatRow(oldID);
       this._reformatRow(newID);
     },
+    availableStatuses() {
+      // Statuses changed, so the filter bar could have gone from "no statuses"
+      // to "any statuses" (or one row of filtering stuff to two, I don't know)
+      // and changed height.
+      this.$nextTick(this.recalcTableHeight);
+    },
   },
   methods: {
     onReconnected() {
@@ -131,7 +138,7 @@ export default {
       this.tabulator.setData(data.tasks);
       this._refreshAvailableStatuses();
 
-      this._setTableHeight();
+      this.recalcTableHeight();
     },
     processTaskUpdate(taskUpdate) {
       // updateData() will only overwrite properties that are actually set on
@@ -180,15 +187,30 @@ export default {
       if (row.reformat) row.reformat();
       else if (row.reinitialize) row.reinitialize(true);
     },
-    _setTableHeight() {
-      const jobDetailsColumn = this.$el.parentElement;
-      const tableContainer = this.tabulator.element.parentElement;
 
-      if (!jobDetailsColumn || !taskListTable) {
+    /**
+     * Recalculate the appropriate table height to fit in the column without making that scroll.
+     */
+    recalcTableHeight() {
+      if (!this.tabulator.initialized) {
+        // Sometimes this function is called too early, before the table was initialised.
+        // After the table is initialised it gets resized anyway, so this call can be ignored.
+        return;
+      }
+      const table = this.tabulator.element;
+      const tableContainer = table.parentElement;
+      const outerContainer = tableContainer.parentElement;
+      const availableHeight = outerContainer.clientHeight - 12; // TODO: figure out where the -12 comes from.
+
+      if (tableContainer.offsetParent != tableContainer.parentElement) {
+        // `offsetParent` is assumed to be the actual column in the 3-column
+        // view. To ensure this, it's given `position: relative` in the CSS
+        // styling.
+        console.warn("TaskTable.recalcTableHeight() only works when the offset parent is the real parent of the element.");
         return;
       }
 
-      let tableHeight = jobDetailsColumn.clientHeight - taskListTable.offsetTop;
+      const tableHeight = availableHeight - tableContainer.offsetTop;
       this.tabulator.setHeight(tableHeight);
     },
   }
diff --git a/web/app/src/views/JobsView.vue b/web/app/src/views/JobsView.vue
index 7f3fd730..4536d9f2 100644
--- a/web/app/src/views/JobsView.vue
+++ b/web/app/src/views/JobsView.vue
@@ -3,7 +3,7 @@
     <jobs-table ref="jobsTable" :activeJobID="jobID" @tableRowClicked="onTableJobClicked" />
   </div>
   <div class="col col-2" id="col-job-details">
-    <job-details :jobData="jobs.activeJob" />
+    <job-details :jobData="jobs.activeJob" @reshuffled="_recalcTasksTableHeight" />
     <tasks-table v-if="hasJobData" ref="tasksTable" :jobID="jobID" :taskID="taskID" @tableRowClicked="onTableTaskClicked" />
   </div>
   <div class="col col-3">
@@ -226,6 +226,12 @@ export default {
     },
     onSIODisconnected(reason) {
     },
+
+    _recalcTasksTableHeight() {
+      if (!this.$refs.tasksTable) return;
+      // Any recalculation should be done after the DOM has updated.
+      this.$nextTick(this.$refs.tasksTable.recalcTableHeight);
+    },
   },
 }
 </script>
@@ -234,4 +240,17 @@ export default {
 .isFetching {
   opacity: 50%;
 }
+
+#col-job-details {
+  /* These two are necessary for the automatic resizing of the tasks table: */
+
+  /* Ensures that the table cannot push down the bottom of the column element,
+  *  and thus the column height is a stable reference. */
+  overflow-y: scroll;
+
+  /* Ensures the offsetParent of the table is the column itself; without this,
+   * offsetParent would be <body>. */
+  position: relative;
+}
+
 </style>