From 5c8211c61d88d80b3b860ee32b8de2bb263a2a9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?=
 <jaroslaw.sadzinski@gmail.com>
Date: Thu, 21 Jul 2022 16:46:26 +0200
Subject: [PATCH] (core) Fixing selectby error on the creator panel.

Summary: [Select By] in the creator panel was bugged. It wasn't refreshed in some cases as the observable array that needed to be created seemed too complicated. This Diff recomputes this array when the user wants to change the selection.

Test Plan: added tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D3541
---
 app/client/ui/RightPanel.ts | 28 ++++++++++++++++++----------
 app/common/DocActions.ts    |  4 ++--
 test/nbrowser/gristUtils.ts |  2 +-
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/app/client/ui/RightPanel.ts b/app/client/ui/RightPanel.ts
index 3c71eb79..21a390a8 100644
--- a/app/client/ui/RightPanel.ts
+++ b/app/client/ui/RightPanel.ts
@@ -396,18 +396,21 @@ export class RightPanel extends Disposable {
       });
     });
 
-    // TODO: this computed is not enough to make sure that the linkOptions are up to date. Indeed
+    // This computed is not enough to make sure that the linkOptions are up to date. Indeed
     // the selectBy function depends on a much greater number of observables. Creating that many
-    // dependencies does not seem a better approach. Instead, we could refresh the list of
-    // linkOptions only when the user clicks the dropdown. Such behaviour is not supported by the
-    // weasel select function as of writing and would require a custom implementation.
-    const linkOptions = Computed.create(owner, (use) =>
-      selectBy(
+    // dependencies does not seem a better approach. Instead, we refresh the list of
+    // linkOptions only when the user clicks on the dropdown. Such behavior is not supported by the
+    // weasel select function as of writing and would require a custom implementation, so we will simulate
+    // this behavior by using temporary observable that will be changed when the user clicks on the dropdown.
+    const refreshTrigger = Observable.create(owner, false);
+    const linkOptions = Computed.create(owner, (use) => {
+      void use(refreshTrigger);
+      return selectBy(
         this._gristDoc.docModel,
-        use(viewModel.viewSections).peek(),
+        viewModel.viewSections().all(),
         activeSection,
-      )
-    );
+      );
+    });
 
     link.onWrite((val) => this._gristDoc.saveLink(val));
     return [
@@ -456,7 +459,12 @@ export class RightPanel extends Disposable {
       dom.maybe((use) => !use(activeSection.isRaw), () => [
         cssLabel('SELECT BY'),
         cssRow(
-          select(link, linkOptions, {defaultLabel: 'Select Widget'}),
+          dom.update(
+            select(link, linkOptions, {defaultLabel: 'Select Widget'}),
+            dom.on('click', () => {
+              refreshTrigger.set(!refreshTrigger.get());
+            })
+          ),
           testId('right-select-by')
         ),
       ]),
diff --git a/app/common/DocActions.ts b/app/common/DocActions.ts
index fc909b18..63d61c87 100644
--- a/app/common/DocActions.ts
+++ b/app/common/DocActions.ts
@@ -189,7 +189,7 @@ export function fromTableDataAction(tableData: TableDataAction): TableColValues
  * Convert a list of rows into an object with columns of values, used for
  * BulkAddRecord/BulkUpdateRecord actions.
  */
-export function getColValues(records: RowRecord[]): BulkColValues {
+export function getColValues(records: Partial<RowRecord>[]): BulkColValues {
   const colIdSet = new Set<string>();
   for (const r of records) {
     for (const c of Object.keys(r)) {
@@ -200,7 +200,7 @@ export function getColValues(records: RowRecord[]): BulkColValues {
   }
   const result: BulkColValues = {};
   for (const colId of colIdSet) {
-    result[colId] = records.map(r => r[colId]);
+    result[colId] = records.map(r => r[colId]!);
   }
   return result;
 }
diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts
index bc6f9032..6b53cfd9 100644
--- a/test/nbrowser/gristUtils.ts
+++ b/test/nbrowser/gristUtils.ts
@@ -1157,7 +1157,7 @@ export function isSidePanelOpen(which: 'right'|'left'): Promise<boolean> {
 }
 
 /*
- * Toggles (opens or closes) the right panel and wait for the transition to complete. An optional
+ * Toggles (opens or closes) the right or left panel and wait for the transition to complete. An optional
  * argument can specify the desired state.
  */
 export async function toggleSidePanel(which: 'right'|'left', goal: 'open'|'close'|'toggle' = 'toggle') {