Code review feedback and improve schedules loading

actualbudget · Oct 18, 2024 · 6a75549 · 6a75549
1 parent 36a3d78
commit 6a75549
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 41 deletions.
diff --git a/packages/desktop-client/src/components/modals/EditRuleModal.jsx b/packages/desktop-client/src/components/modals/EditRuleModal.jsx
@@ -303,13 +303,15 @@ function formatAmount(amount) {
 
 function ScheduleDescription({ id }) {
   const dateFormat = useDateFormat() || 'MM/dd/yyyy';
+  const scheduleQuery = useMemo(
+    () => q('schedules').filter({ id }).select('*'),
+    [id],
+  );
   const {
     schedules,
     statuses: scheduleStatuses,
     isLoading: isSchedulesLoading,
-  } = useSchedules({
-    query: useMemo(() => q('schedules').filter({ id }).select('*'), [id]),
-  });
+  } = useSchedules({ query: scheduleQuery });
 
   if (isSchedulesLoading) {
     return null;

diff --git a/packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal.tsx b/packages/desktop-client/src/components/modals/ScheduledTransactionMenuModal.tsx
@@ -33,11 +33,12 @@ export function ScheduledTransactionMenuModal({
     borderTop: `1px solid ${theme.pillBorder}`,
   };
   const scheduleId = transactionId?.split('/')?.[1];
+  const schedulesQuery = useMemo(
+    () => q('schedules').filter({ id: scheduleId }).select('*'),
+    [scheduleId],
+  );
   const { isLoading: isSchedulesLoading, schedules } = useSchedules({
-    query: useMemo(
-      () => q('schedules').filter({ id: scheduleId }).select('*'),
-      [scheduleId],
-    ),
+    query: schedulesQuery,
   });
 
   if (isSchedulesLoading) {

diff --git a/packages/desktop-client/src/components/rules/ScheduleValue.tsx b/packages/desktop-client/src/components/rules/ScheduleValue.tsx
@@ -7,6 +7,8 @@ import { describeSchedule } from 'loot-core/src/shared/schedules';
 import { type ScheduleEntity } from 'loot-core/src/types/models';
 
 import { usePayees } from '../../hooks/usePayees';
+import { AnimatedLoading } from '../../icons/AnimatedLoading';
+import { View } from '../common/View';
 
 import { Value } from './Value';
 
@@ -17,12 +19,15 @@ type ScheduleValueProps = {
 export function ScheduleValue({ value }: ScheduleValueProps) {
   const payees = usePayees();
   const byId = getPayeesById(payees);
-  const { schedules = [], isLoading } = useSchedules({
-    query: useMemo(() => q('schedules').select('*'), []),
-  });
+  const schedulesQuery = useMemo(() => q('schedules').select('*'), []);
+  const { schedules = [], isLoading } = useSchedules({ query: schedulesQuery });
 
   if (isLoading) {
-    return null;
+    return (
+      <View aria-label="Loading..." style={{ display: 'inline-flex' }}>
+        <AnimatedLoading width={10} height={10} />
+      </View>
+    );
   }
 
   return (

diff --git a/packages/desktop-client/src/components/schedules/ScheduleLink.tsx b/packages/desktop-client/src/components/schedules/ScheduleLink.tsx
@@ -37,20 +37,17 @@ export function ScheduleLink({
 
   const dispatch = useDispatch();
   const [filter, setFilter] = useState(accountName || '');
-
-  const scheduleData = useSchedules({
-    query: useMemo(
-      () => q('schedules').filter({ completed: false }).select('*'),
-      [],
-    ),
-  });
+  const schedulesQuery = useMemo(
+    () => q('schedules').filter({ completed: false }).select('*'),
+    [],
+  );
+  const {
+    isLoading: isSchedulesLoading,
+    schedules,
+    statuses,
+  } = useSchedules({ query: schedulesQuery });
 
   const searchInput = useRef(null);
-  if (scheduleData == null) {
-    return null;
-  }
-
-  const { schedules, statuses } = scheduleData;
 
   async function onSelect(scheduleId: string) {
     if (ids?.length > 0) {
@@ -134,6 +131,7 @@ export function ScheduleLink({
             }}
           >
             <SchedulesTable
+              isLoading={isSchedulesLoading}
               allowCompleted={false}
               filter={filter}
               minimal={true}

diff --git a/packages/desktop-client/src/components/schedules/SchedulesTable.tsx b/packages/desktop-client/src/components/schedules/SchedulesTable.tsx
@@ -30,6 +30,7 @@ import { DisplayId } from '../util/DisplayId';
 import { StatusBadge } from './StatusBadge';
 
 type SchedulesTableProps = {
+  isLoading?: boolean;
   schedules: readonly ScheduleEntity[];
   statuses: ScheduleStatuses;
   filter: string;
@@ -197,6 +198,7 @@ export function ScheduleAmountCell({
 }
 
 export function SchedulesTable({
+  isLoading,
   schedules,
   statuses,
   filter,
@@ -385,6 +387,7 @@ export function SchedulesTable({
         {!minimal && <Field width={40} />}
       </TableHeader>
       <Table
+        loading={isLoading}
         rowHeight={ROW_HEIGHT}
         backgroundColor="transparent"
         style={{ flex: 1, backgroundColor: 'transparent', ...style }}

diff --git a/packages/desktop-client/src/components/schedules/index.tsx b/packages/desktop-client/src/components/schedules/index.tsx
@@ -68,17 +68,12 @@ export function Schedules() {
     [],
   );
 
+  const schedulesQuery = useMemo(() => q('schedules').select('*'), []);
   const {
     isLoading: isSchedulesLoading,
     schedules,
     statuses,
-  } = useSchedules({
-    query: useMemo(() => q('schedules').select('*'), []),
-  });
-
-  if (isSchedulesLoading) {
-    return null;
-  }
+  } = useSchedules({ query: schedulesQuery });
 
   return (
     <Page header={t('Schedules')}>
@@ -114,6 +109,7 @@ export function Schedules() {
       </View>
 
       <SchedulesTable
+        isLoading={isSchedulesLoading}
         schedules={schedules}
         filter={filter}
         statuses={statuses}

diff --git a/packages/loot-core/src/client/data-hooks/filters.ts b/packages/loot-core/src/client/data-hooks/filters.ts
@@ -1,5 +1,5 @@
 // @ts-strict-ignore
-import { useMemo, useCallback } from 'react';
+import { useMemo } from 'react';
 
 import { q } from '../../shared/query';
 import { type TransactionFilterEntity } from '../../types/models';
@@ -25,14 +25,13 @@ export function useFilters(): TransactionFilterEntity[] {
     [],
   );
 
-  /** Sort filters by alphabetical order */
-  const sort = useCallback((filters: TransactionFilterEntity[]) => {
-    return [...filters].sort((a, b) =>
-      a.name
-        .trim()
-        .localeCompare(b.name.trim(), undefined, { ignorePunctuation: true }),
-    );
-  }, []);
-
-  return useMemo(() => sort(toJS(data ? [...data] : [])), [data, sort]);
+  return useMemo(
+    () =>
+      toJS(data ? [...data] : []).sort((a, b) =>
+        a.name
+          .trim()
+          .localeCompare(b.name.trim(), undefined, { ignorePunctuation: true }),
+      ),
+    [data],
+  );
 }