From c1128618fbdd0e59a640747d2d6e00f972b54175 Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Sat, 11 Mar 2023 18:28:58 +0000
Subject: [PATCH] Fix bug with select placeholders and fix sorting loops
 causing endless refreshes

---
 packages/bbui/src/Form/Core/Select.svelte     |  5 ++-
 .../src/components/sheet/StickyColumn.svelte  |  6 ++--
 .../sheet/controls/SortButton.svelte          | 34 ++++++++++++++++---
 .../src/components/sheet/stores/rows.js       | 30 +++++-----------
 .../src/components/sheet/stores/scroll.js     |  8 ++---
 5 files changed, 50 insertions(+), 33 deletions(-)

diff --git a/packages/bbui/src/Form/Core/Select.svelte b/packages/bbui/src/Form/Core/Select.svelte
index af45c1d9ff..6e50cfd1f4 100644
--- a/packages/bbui/src/Form/Core/Select.svelte
+++ b/packages/bbui/src/Form/Core/Select.svelte
@@ -44,7 +44,10 @@
   const getFieldText = (value, options, placeholder) => {
     // Always use placeholder if no value
     if (value == null || value === "") {
-      return placeholder !== false ? "Choose an option" : ""
+      if (placeholder === false) {
+        return ""
+      }
+      return placeholder || "Choose an option"
     }
 
     return getFieldAttribute(getOptionLabel, value, options)
diff --git a/packages/frontend-core/src/components/sheet/StickyColumn.svelte b/packages/frontend-core/src/components/sheet/StickyColumn.svelte
index c53d587d5b..3416173b53 100644
--- a/packages/frontend-core/src/components/sheet/StickyColumn.svelte
+++ b/packages/frontend-core/src/components/sheet/StickyColumn.svelte
@@ -59,14 +59,16 @@
   class:scrolled={scrollLeft > 0}
 >
   <div class="header row">
-    <!-- Field headers -->
     <SheetCell
       width="40"
       on:click={$config.allowSelectRows && selectAll}
       center
     >
       {#if $config.allowSelectRows}
-        <Checkbox value={rowCount && selectedRowCount === rowCount} />
+        <Checkbox
+          value={rowCount && selectedRowCount === rowCount}
+          disabled={!$renderedRows.length}
+        />
       {/if}
     </SheetCell>
 
diff --git a/packages/frontend-core/src/components/sheet/controls/SortButton.svelte b/packages/frontend-core/src/components/sheet/controls/SortButton.svelte
index 2e44dcdc35..e84bd45087 100644
--- a/packages/frontend-core/src/components/sheet/controls/SortButton.svelte
+++ b/packages/frontend-core/src/components/sheet/controls/SortButton.svelte
@@ -22,6 +22,20 @@
     return [...options, ...columns.map(col => col.name)]
   }
 
+  const updateSortColumn = e => {
+    sort.update(state => ({
+      ...state,
+      column: e.detail,
+    }))
+  }
+
+  const updateSortOrder = e => {
+    sort.update(state => ({
+      ...state,
+      order: e.detail,
+    }))
+  }
+
   // Ensure we never have a sort column selected that is not visible
   const checkValidSortColumn = (sortColumn, stickyColumn, visibleColumns) => {
     if (!sortColumn) {
@@ -52,8 +66,8 @@
     quiet
     size="M"
     on:click={() => (open = !open)}
-    selected={!!$sort.column}
-    disabled={!$visibleColumns.length}
+    selected={open || $sort.column}
+    disabled={!columnOptions.length}
   >
     Sort
   </ActionButton>
@@ -61,8 +75,20 @@
 
 <Popover bind:open {anchor} align="left">
   <div class="content">
-    <Select bind:value={$sort.column} options={columnOptions} autoWidth />
-    <Select bind:value={$sort.order} options={orderOptions} autoWidth />
+    <Select
+      placeholder="Default"
+      value={$sort.column}
+      options={columnOptions}
+      autoWidth
+      on:change={updateSortColumn}
+    />
+    <Select
+      placeholder={null}
+      value={$sort.order}
+      options={orderOptions}
+      autoWidth
+      on:change={updateSortOrder}
+    />
   </div>
 </Popover>
 
diff --git a/packages/frontend-core/src/components/sheet/stores/rows.js b/packages/frontend-core/src/components/sheet/stores/rows.js
index c9219ffdc1..14d056efdb 100644
--- a/packages/frontend-core/src/components/sheet/stores/rows.js
+++ b/packages/frontend-core/src/components/sheet/stores/rows.js
@@ -11,10 +11,11 @@ export const createRowsStore = context => {
   const filter = writable([])
   const loaded = writable(false)
   const fetch = writable(null)
-  const sort = writable({
+  const initialSortState = {
     column: null,
-    order: null,
-  })
+    order: "ascending",
+  }
+  const sort = writable(initialSortState)
 
   // Enrich rows with an index property
   const enrichedRows = derived(rows, $rows => {
@@ -44,10 +45,7 @@ export const createRowsStore = context => {
     fetch.set(null)
 
     // Reset state
-    sort.set({
-      column: null,
-      order: null,
-    })
+    sort.set(initialSortState)
     filter.set([])
 
     // Create new fetch model
@@ -59,8 +57,8 @@ export const createRowsStore = context => {
       },
       options: {
         filter: [],
-        sortColumn: null,
-        sortOrder: null,
+        sortColumn: initialSortState.column,
+        sortOrder: initialSortState.order,
         limit: 100,
         paginate: true,
       },
@@ -73,15 +71,6 @@ export const createRowsStore = context => {
           // Hydrate initial data
           rowCacheMap = {}
           rows.set([])
-
-          // Update sorting from fetch if required
-          const $sort = get(sort)
-          if (!$sort.column) {
-            sort.set({
-              column: $fetch.sortColumn,
-              order: $fetch.sortOrder,
-            })
-          }
         }
 
         // Update schema and enrich primary display into schema
@@ -180,10 +169,7 @@ export const createRowsStore = context => {
   // Refreshes all data
   const refreshData = () => {
     filter.set([])
-    sort.set({
-      column: null,
-      order: null,
-    })
+    sort.set(initialSortState)
   }
 
   // Updates a value of a row
diff --git a/packages/frontend-core/src/components/sheet/stores/scroll.js b/packages/frontend-core/src/components/sheet/stores/scroll.js
index e708fbd3d6..599f16770b 100644
--- a/packages/frontend-core/src/components/sheet/stores/scroll.js
+++ b/packages/frontend-core/src/components/sheet/stores/scroll.js
@@ -1,7 +1,7 @@
 import { derived, get, writable } from "svelte/store"
 
 export const createScrollStores = context => {
-  const { rows, columns, stickyColumn, bounds, cellHeight } = context
+  const { rows, visibleColumns, stickyColumn, bounds, cellHeight } = context
   const padding = 180
   const scroll = writable({
     left: 0,
@@ -28,10 +28,10 @@ export const createScrollStores = context => {
 
   // Derive horizontal limits
   const contentWidth = derived(
-    [columns, stickyColumn],
-    ([$columns, $stickyColumn]) => {
+    [visibleColumns, stickyColumn],
+    ([$visibleColumns, $stickyColumn]) => {
       let width = 40 + padding + ($stickyColumn?.width || 0)
-      $columns.forEach(col => {
+      $visibleColumns.forEach(col => {
         width += col.width
       })
       return width