From 1310e5c9e27343a2a1249ff9f5772a01b10e0a85 Mon Sep 17 00:00:00 2001
From: Pablo Vazquez <pablo@blender.org>
Date: Wed, 25 May 2022 17:14:21 +0200
Subject: [PATCH 1/6] Web: Set height for Tasks List on load and window resize

Calculate the Tasks List height by subtracting its offsetTop from
the Jobs Details column height.

Known issues:
There is a glitch in which the height can be sometimes longer or shorter
than expected, due to asynchronous loading of other components in Job Details
(such as Job Settings or table filters).
---
 web/app/src/components/TasksTable.vue | 26 ++++++++++++++++++++++++--
 web/app/src/views/JobsView.vue        |  2 +-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/web/app/src/components/TasksTable.vue b/web/app/src/components/TasksTable.vue
index 334cc784..f87ee6d8 100644
--- a/web/app/src/components/TasksTable.vue
+++ b/web/app/src/components/TasksTable.vue
@@ -5,7 +5,7 @@
     :activeStatuses="shownStatuses"
     @click="toggleStatusFilter"
   />
-  <div class="task-list-container">
+  <div class="task-list-container" id="task-list-container">
     <div class="task-list" id="flamenco_task_list"></div>
   </div>
 </template>
@@ -74,7 +74,8 @@ export default {
       initialSort: [
         { column: "updated", dir: "desc" },
       ],
-      height: "300px", // Must be set in order for the virtual DOM to function correctly.
+      height: "100%", // Must be set in order for the virtual DOM to function correctly.
+      maxHeight: "100%",
       data: [], // Will be filled via a Flamenco API request.
       selectable: false, // The active task is tracked by click events.
     };
@@ -124,6 +125,8 @@ export default {
       // let tasks = data.tasks.map((j) => API.TaskUpdate.constructFromObject(j));
       this.tabulator.setData(data.tasks);
       this._refreshAvailableStatuses();
+
+      this._setTableHeight();
     },
     processTaskUpdate(taskUpdate) {
       // updateData() will only overwrite properties that are actually set on
@@ -171,7 +174,26 @@ export default {
       if (!row) return
       if (row.reformat) row.reformat();
       else if (row.reinitialize) row.reinitialize(true);
+    },
+    _setTableHeight() {
+      let jobDetailsColumn = document.getElementById('col-job-details');
+      let taskListTable = document.getElementById('task-list-container');
+
+      if (!jobDetailsColumn || !taskListTable) {
+        return;
+      }
+
+      let tableHeight = jobDetailsColumn.clientHeight - taskListTable.offsetTop;
+      this.tabulator.setHeight(tableHeight);
     }
   }
 };
+
+function resizeTasksListTable() {
+  if (window.tasksTableVue) {
+    window.tasksTableVue._setTableHeight();
+  }
+}
+
+window.addEventListener('resize', resizeTasksListTable);
 </script>
diff --git a/web/app/src/views/JobsView.vue b/web/app/src/views/JobsView.vue
index 5dfc4665..7f3fd730 100644
--- a/web/app/src/views/JobsView.vue
+++ b/web/app/src/views/JobsView.vue
@@ -2,7 +2,7 @@
   <div class="col col-1">
     <jobs-table ref="jobsTable" :activeJobID="jobID" @tableRowClicked="onTableJobClicked" />
   </div>
-  <div class="col col-2">
+  <div class="col col-2" id="col-job-details">
     <job-details :jobData="jobs.activeJob" />
     <tasks-table v-if="hasJobData" ref="tasksTable" :jobID="jobID" :taskID="taskID" @tableRowClicked="onTableTaskClicked" />
   </div>

From 60836c77e0e48891f126368f3ea12ed6ddbadde2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Fri, 27 May 2022 16:43:12 +0200
Subject: [PATCH 2/6] Remove need for global `resizeTasksListTable` function

---
 web/app/src/components/TasksTable.vue | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/web/app/src/components/TasksTable.vue b/web/app/src/components/TasksTable.vue
index f87ee6d8..11d86eef 100644
--- a/web/app/src/components/TasksTable.vue
+++ b/web/app/src/components/TasksTable.vue
@@ -83,6 +83,11 @@ export default {
     this.tabulator = new Tabulator('#flamenco_task_list', options);
     this.tabulator.on("rowClick", this.onRowClick);
     this.tabulator.on("tableBuilt", this._onTableBuilt);
+
+    window.addEventListener('resize', this._setTableHeight);
+  },
+  unmounted() {
+    window.removeEventListener('resize', this._setTableHeight);
   },
   watch: {
     jobID() {
@@ -189,11 +194,4 @@ export default {
   }
 };
 
-function resizeTasksListTable() {
-  if (window.tasksTableVue) {
-    window.tasksTableVue._setTableHeight();
-  }
-}
-
-window.addEventListener('resize', resizeTasksListTable);
 </script>

From 339aafc1e49c45992bf814408e0160ab2900c19b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Fri, 27 May 2022 16:43:40 +0200
Subject: [PATCH 3/6] Cleanup: add missing trailing comma

This comma isn't strictly necessary, but makes the diff cleaner when adding
new functions.
---
 web/app/src/components/TasksTable.vue | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/web/app/src/components/TasksTable.vue b/web/app/src/components/TasksTable.vue
index 11d86eef..25b09e3e 100644
--- a/web/app/src/components/TasksTable.vue
+++ b/web/app/src/components/TasksTable.vue
@@ -190,7 +190,7 @@ export default {
 
       let tableHeight = jobDetailsColumn.clientHeight - taskListTable.offsetTop;
       this.tabulator.setHeight(tableHeight);
-    }
+    },
   }
 };
 

From 09dbcfe51264e24a7cb558a3504753c3a2e2c285 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Fri, 27 May 2022 16:44:51 +0200
Subject: [PATCH 4/6] Remove the need to get elements by their ID

Referencing elements outside the current component by ID is not a good
approach; using `this.$el.parentElement` is a bit cleaner.
---
 web/app/src/components/TasksTable.vue | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/web/app/src/components/TasksTable.vue b/web/app/src/components/TasksTable.vue
index 25b09e3e..f35ca920 100644
--- a/web/app/src/components/TasksTable.vue
+++ b/web/app/src/components/TasksTable.vue
@@ -5,7 +5,7 @@
     :activeStatuses="shownStatuses"
     @click="toggleStatusFilter"
   />
-  <div class="task-list-container" id="task-list-container">
+  <div class="tabulator-container">
     <div class="task-list" id="flamenco_task_list"></div>
   </div>
 </template>
@@ -181,8 +181,8 @@ export default {
       else if (row.reinitialize) row.reinitialize(true);
     },
     _setTableHeight() {
-      let jobDetailsColumn = document.getElementById('col-job-details');
-      let taskListTable = document.getElementById('task-list-container');
+      const jobDetailsColumn = this.$el.parentElement;
+      const tableContainer = this.tabulator.element.parentElement;
 
       if (!jobDetailsColumn || !taskListTable) {
         return;

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 5/6] 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>

From 74f875957fb1a28ee8a8f6537d613e12199082f3 Mon Sep 17 00:00:00 2001
From: Pablo Vazquez <pablo@blender.org>
Date: Mon, 30 May 2022 15:44:38 +0200
Subject: [PATCH 6/6] Web: Move overflow and position properties to affect all
 columns

---
 web/app/src/assets/base.css    |  8 ++++++++
 web/app/src/views/JobsView.vue | 13 -------------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/web/app/src/assets/base.css b/web/app/src/assets/base.css
index 19a37d50..2178a132 100644
--- a/web/app/src/assets/base.css
+++ b/web/app/src/assets/base.css
@@ -101,6 +101,14 @@ body {
   background-color: var(--color-background-column);
   border-radius: var(--border-radius);
   padding: var(--spacer-sm);
+
+  /* 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: auto;
+  /* Ensures the offsetParent of the table is the column itself; without this,
+  * offsetParent would be <body>. */
+  position: relative;
 }
 
 .col-1 {
diff --git a/web/app/src/views/JobsView.vue b/web/app/src/views/JobsView.vue
index 4536d9f2..8f2d30e2 100644
--- a/web/app/src/views/JobsView.vue
+++ b/web/app/src/views/JobsView.vue
@@ -240,17 +240,4 @@ 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>