From 524d28841822830fc77779b8e98f175b835a68e5 Mon Sep 17 00:00:00 2001
From: Charlie Drage <charlie@charliedrage.com>
Date: Sun, 9 Jun 2024 11:08:47 -0400
Subject: [PATCH] chore: keep sorting despite data changes (#7522)

---
 packages/ui/src/lib/table/Table.spec.ts    | 52 ++++++++++++++++++++++
 packages/ui/src/lib/table/Table.svelte     |  2 +-
 packages/ui/src/lib/table/TestTable.svelte |  2 +-
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/packages/ui/src/lib/table/Table.spec.ts b/packages/ui/src/lib/table/Table.spec.ts
index 0d3ca9078e2..a5845c0f8d2 100644
--- a/packages/ui/src/lib/table/Table.spec.ts
+++ b/packages/ui/src/lib/table/Table.spec.ts
@@ -19,6 +19,7 @@
 import '@testing-library/jest-dom/vitest';
 
 import { fireEvent, render, screen, within } from '@testing-library/svelte';
+import { tick } from 'svelte';
 import { expect, test, vi } from 'vitest';
 
 import TestTable from './TestTable.svelte';
@@ -286,3 +287,54 @@ test('Expect table to be sorted by Id on load', async () => {
     expect(rows[i].textContent).toContain(i.toString());
   }
 });
+
+test('Expect table to be sorted by Name on load, if something has changed in the store afterwards, it should not affect the order', async () => {
+  const result = render(TestTable, {});
+
+  const nameCol = screen.getByText('Name');
+  expect(nameCol).toBeInTheDocument();
+
+  let rows = await screen.findAllByRole('row');
+  expect(rows).toBeDefined();
+  expect(rows.length).toBe(4);
+  expect(rows[1].textContent).toContain('John');
+  expect(rows[2].textContent).toContain('Henry');
+  expect(rows[3].textContent).toContain('Charlie');
+
+  await fireEvent.click(nameCol);
+
+  rows = await screen.findAllByRole('row');
+  expect(rows[1].textContent).toContain('Charlie');
+  expect(rows[2].textContent).toContain('Henry');
+  expect(rows[3].textContent).toContain('John');
+
+  // Change the store, this is similar to what is already in TestTable, but updates the hobbies of all the rows.
+  // The original is:
+  //
+  //  { id: 1, name: 'John', age: 57, hobby: 'Skydiving' },
+  //  { id: 2, name: 'Henry', age: 27, hobby: 'Cooking' },
+  //  { id: 3, name: 'Charlie', age: 43, hobby: 'Biking' },
+  //
+
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const people: any[] = [
+    { id: 1, name: 'John', age: 57, hobby: 'Walking' },
+    { id: 2, name: 'Henry', age: 27, hobby: 'Swimming' },
+    { id: 3, name: 'Charlie', age: 43, hobby: 'Karting' },
+  ];
+  result.component.$set({ people });
+
+  // Wait for the table to update
+  await tick();
+
+  // Check that the order is still the same even though hobbies had changed above.
+  const newRows = await screen.findAllByRole('row');
+  expect(newRows).toBeDefined();
+  expect(newRows.length).toBe(4);
+  expect(newRows[1].textContent).toContain('Charlie');
+  expect(newRows[1].textContent).toContain('Karting');
+  expect(newRows[2].textContent).toContain('Henry');
+  expect(newRows[2].textContent).toContain('Swimming');
+  expect(newRows[3].textContent).toContain('John');
+  expect(newRows[3].textContent).toContain('Walking');
+});
diff --git a/packages/ui/src/lib/table/Table.svelte b/packages/ui/src/lib/table/Table.svelte
index 3035945240a..58279b40be6 100644
--- a/packages/ui/src/lib/table/Table.svelte
+++ b/packages/ui/src/lib/table/Table.svelte
@@ -50,7 +50,7 @@ function toggleAll(e: CustomEvent<boolean>): void {
 let sortCol: Column<unknown>;
 let sortAscending: boolean;
 
-if (data) {
+$: if (data && sortCol) {
   sortImpl();
 }
 
diff --git a/packages/ui/src/lib/table/TestTable.svelte b/packages/ui/src/lib/table/TestTable.svelte
index 74dd9e352b3..fd0dd8740c3 100644
--- a/packages/ui/src/lib/table/TestTable.svelte
+++ b/packages/ui/src/lib/table/TestTable.svelte
@@ -17,7 +17,7 @@ type Person = {
   hobby: string;
 };
 
-const people: Person[] = [
+export let people: Person[] = [
   { id: 1, name: 'John', age: 57, hobby: 'Skydiving' },
   { id: 2, name: 'Henry', age: 27, hobby: 'Cooking' },
   { id: 3, name: 'Charlie', age: 43, hobby: 'Biking' },