UX: Tweak JS height calculation logic (#42)

- Moves the height calculation to the wrapper element so we don't need to re-calculate for every list - Instead of enumerating the sibling elements, calculate an `--kanban-offset` value for the top of the wrapper, and then use css viewport height to work out how much space is remaining - Set a minimum of 500px so that the kanban is still usable on sites with very large headers/footers.
discourse · Dec 22, 2023 · 989c34f · 989c34f
1 parent 8d92616
commit 989c34f
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 77 deletions.
diff --git a/common/common.scss b/common/common.scss
@@ -1,14 +1,23 @@
 html:has(body.kanban-active) {
   overscroll-behavior: none;
+
+  #list-area {
+    margin-bottom: 0;
+  }
 }
 
 .discourse-kanban {
   position: relative;
-  height: 100%;
-  --d-sidebar-gap: 2em;
-  @media screen and (max-width: 1000px) {
-    --d-sidebar-gap: 1em;
-  }
+
+  // The amount of space available without causing the page to scroll:
+  --desired-height: calc(100vh - var(--kanban-offset-top));
+
+  // Use the desired height, unless it is smaller than 500px
+  height: max(var(--desired-height), 500px);
+
+  padding-bottom: 10px;
+  overflow-x: scroll;
+  box-sizing: border-box;
 
   &.kanban-fullscreen {
     position: fixed;
@@ -19,10 +28,10 @@ html:has(body.kanban-active) {
     z-index: z("fullscreen");
     background-color: var(--secondary);
     width: 100%;
+    height: 100%;
     padding: 0;
     margin: 0;
     display: flex;
-    justify-content: center;
 
     .discourse-kanban-container {
       height: auto;
@@ -59,12 +68,13 @@ html:has(body.kanban-active) {
 .discourse-kanban-container {
   height: 100%;
   display: flex;
-  overflow-y: scroll;
   gap: 0 10px;
 
   .discourse-kanban-list {
     background: var(--primary-100);
     border: 1px solid var(--primary-low);
+    display: flex;
+    flex-direction: column;
 
     &.kanban-empty-state {
       padding: 2em 1em;
@@ -91,6 +101,7 @@ html:has(body.kanban-active) {
     .topics {
       overflow-y: scroll;
       padding: 0 8px;
+      height: 100%;
     }
 
     .topic-card {
@@ -206,21 +217,6 @@ html:has(body.kanban-active) {
   }
 }
 
-body.kanban-active {
-  #main-outlet {
-    height: calc(100dvh - var(--header-offset) - 2.5em);
-  }
-
-  .list-container .row:nth-child(2),
-  .full-width {
-    height: 100%;
-  }
-
-  #list-area {
-    height: 100%;
-  }
-}
-
 ul.kanban-controls {
   padding: 0.5rem;
   margin: 0;

diff --git a/javascripts/discourse/components/kanban/list.gjs b/javascripts/discourse/components/kanban/list.gjs
@@ -2,10 +2,8 @@ import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
 import { on } from "@ember/modifier";
 import { action } from "@ember/object";
-import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import { inject as service } from "@ember/service";
 import { modifier } from "ember-modifier";
-import ConditionalLoadingSpinner from "discourse/components/conditional-loading-spinner";
 import concatClass from "discourse/helpers/concat-class";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import Topic from "discourse/models/topic";
@@ -14,15 +12,6 @@ import i18n from "discourse-common/helpers/i18n";
 import I18n from "I18n";
 import DiscourseKanbanCard from "./card";
 
-export const onWindowReize = modifier((element, [callback]) => {
-  const wrappedCallback = () => callback(element);
-  window.addEventListener("resize", wrappedCallback);
-
-  return () => {
-    window.removeEventListener("resize", wrappedCallback);
-  };
-});
-
 function removedElements(before, after) {
   if (!before) {
     return [];
@@ -296,12 +285,10 @@ export default class KanbanList extends Component {
         {{this.renderedTitle}}
       </div>
 
-      <ConditionalLoadingSpinner @condition={{this.loading}}>
-        <div
-          class="topics"
-          {{onWindowReize this.kanbanManager.calcListsHeights}}
-          {{didInsert this.kanbanManager.calcListsHeights}}
-        >
+      {{#if this.loading}}
+        <div class="spinner"></div>
+      {{else}}
+        <div class="topics">
           {{#each this.list.topics as |topic|}}
             <DiscourseKanbanCard
               @topic={{topic}}
@@ -316,7 +303,7 @@ export default class KanbanList extends Component {
 
           <div class="list-bottom" {{onIntersection this.loadMore}}></div>
         </div>
-      </ConditionalLoadingSpinner>
+      {{/if}}
     </div>
   </template>
 }
diff --git a/javascripts/discourse/components/kanban/wrapper.gjs b/javascripts/discourse/components/kanban/wrapper.gjs
@@ -1,14 +1,35 @@
 import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
+import didInsert from "@ember/render-modifiers/modifiers/did-insert";
+import { schedule } from "@ember/runloop";
 import { inject as service } from "@ember/service";
+import { modifier } from "ember-modifier";
 import DButton from "discourse/components/d-button";
 import bodyClass from "discourse/helpers/body-class";
 import concatClass from "discourse/helpers/concat-class";
 import i18n from "discourse-common/helpers/i18n";
 import DiscourseKanbanList from "./list";
 import KanbanOptionsModal from "./modal/options";
 
+const onWindowResize = modifier((element, [callback]) => {
+  const wrappedCallback = () => callback(element);
+  window.addEventListener("resize", wrappedCallback);
+
+  return () => {
+    window.removeEventListener("resize", wrappedCallback);
+  };
+});
+
+function calcOffset(element) {
+  schedule("afterRender", () => {
+    element.style.setProperty(
+      "--kanban-offset-top",
+      `${element.getBoundingClientRect().top}px`
+    );
+  });
+}
+
 export default class Kanban extends Component {
   @service kanbanManager;
   @service modal;
@@ -38,6 +59,8 @@ export default class Kanban extends Component {
           "discourse-kanban"
           (if this.kanbanManager.fullscreen "kanban-fullscreen" "kanban-inline")
         }}
+        {{onWindowResize calcOffset}}
+        {{didInsert calcOffset}}
       >
         {{#if this.kanbanManager.fullscreen}}
           <div class="fullscreen-close-wrapper">

diff --git a/javascripts/discourse/services/kanban-manager.js b/javascripts/discourse/services/kanban-manager.js
@@ -1,5 +1,5 @@
 import { tracked } from "@glimmer/tracking";
-import { action, get } from "@ember/object";
+import { get } from "@ember/object";
 import Service, { inject as service } from "@ember/service";
 import Category from "discourse/models/category";
 import buildAssignedLists from "../lib/kanban-list-builders/assigned";
@@ -120,42 +120,6 @@ export default class KanbanManager extends Service {
     return this.resolvedDescriptorParts[0];
   }
 
-  @action
-  calcListsHeights() {
-    const mainOutlet = document.querySelector("#main-outlet");
-    const mainOutletHeight = mainOutlet.getBoundingClientRect().height;
-    const mainOutletPadding = 40;
-
-    // Get all previous siblings of the list container and add their heights
-    let currentElement =
-      mainOutlet.querySelector(".list-container").previousElementSibling;
-    let previousSiblingsHeight = 0;
-    while (currentElement !== null) {
-      previousSiblingsHeight += currentElement.getBoundingClientRect().height;
-      currentElement = currentElement.previousElementSibling;
-    }
-
-    const listTitleHeight = mainOutlet
-      .querySelector(".list-title")
-      .getBoundingClientRect().height;
-    let height;
-    if (this.fullscreen) {
-      // the 10px is for the padding on the top of the list
-      height = mainOutletHeight + 10;
-    } else {
-      height =
-        mainOutletHeight -
-        previousSiblingsHeight -
-        listTitleHeight -
-        mainOutletPadding;
-    }
-
-    const lists = document.querySelectorAll(".discourse-kanban-list .topics");
-    lists.forEach((element) => {
-      element.style.height = `${height}px`;
-    });
-  }
-
   findDefinition() {
     const [type, param] = this.resolvedDescriptorParts;