Mobile rules - refactor to use react-aria GridList component (#5804)

packages/desktop-client/e2e/page-models/mobile-rules-page.ts

@@ -40,17 +40,14 @@ export class MobileRulesPage {
    * Get the nth rule item (0-based index)
    */
   getNthRule(index: number) {
-    return this.page
-      .getByRole('button')
-      .filter({ hasText: /IF|THEN/ })
-      .nth(index);
+    return this.getAllRules().nth(index);
   }
 
   /**
    * Get all visible rule items
    */
   getAllRules() {
-    return this.page.getByRole('button').filter({ hasText: /IF|THEN/ });
+    return this.page.getByRole('grid', { name: 'Rules' }).getByRole('row');
   }
 
   /**
@@ -112,7 +109,7 @@ export class MobileRulesPage {
    */
   async getRuleStage(index: number) {
     const rule = this.getNthRule(index);
-    const stageBadge = rule.locator('span').first();
+    const stageBadge = rule.getByTestId('rule-stage-badge');
     return await stageBadge.textContent();
   }
 }

packages/desktop-client/src/components/mobile/rules/MobileRulesPage.tsx

@@ -23,15 +23,12 @@ import { usePayees } from '@desktop-client/hooks/usePayees';
 import { useSchedules } from '@desktop-client/hooks/useSchedules';
 import { useUrlParam } from '@desktop-client/hooks/useUrlParam';
 
-const PAGE_SIZE = 50;
-
 export function MobileRulesPage() {
   const { t } = useTranslation();
   const navigate = useNavigate();
   const [visibleRulesParam] = useUrlParam('visible-rules');
   const [allRules, setAllRules] = useState<RuleEntity[]>([]);
   const [isLoading, setIsLoading] = useState(true);
-  const [hasMoreRules, setHasMoreRules] = useState(true);
   const [filter, setFilter] = useState('');
 
   const { schedules = [] } = useSchedules({
@@ -76,16 +73,12 @@ export function MobileRulesPage() {
         );
   }, [visibleRules, filter, filterData, schedules]);
 
-  const loadRules = useCallback(async (append = false) => {
+  const loadRules = useCallback(async () => {
     try {
       setIsLoading(true);
       const result = await send('rules-get');
-      const newRules = result || [];
-
-      setAllRules(prevRules =>
-        append ? [...prevRules, ...newRules] : newRules,
-      );
-      setHasMoreRules(newRules.length === PAGE_SIZE);
+      const rules = result || [];
+      setAllRules(rules);
     } catch (error) {
       console.error('Failed to load rules:', error);
       setAllRules([]);
@@ -105,12 +98,6 @@ export function MobileRulesPage() {
     [navigate],
   );
 
-  const handleLoadMore = useCallback(() => {
-    if (!isLoading && hasMoreRules && !filter) {
-      loadRules(true);
-    }
-  }, [isLoading, hasMoreRules, filter, loadRules]);
-
   const onSearchChange = useCallback(
     (value: string) => {
       setFilter(value);
@@ -153,7 +140,6 @@ export function MobileRulesPage() {
         rules={filteredRules}
         isLoading={isLoading}
         onRulePress={handleRulePress}
-        onLoadMore={handleLoadMore}
       />
     </Page>
   );

packages/desktop-client/src/components/mobile/rules/RulesList.tsx

@@ -1,4 +1,4 @@
-import { type UIEvent } from 'react';
+import { GridList } from 'react-aria-components';
 import { useTranslation } from 'react-i18next';
 
 import { AnimatedLoading } from '@actual-app/components/icons/AnimatedLoading';
@@ -16,15 +16,9 @@ type RulesListProps = {
   rules: RuleEntity[];
   isLoading: boolean;
   onRulePress: (rule: RuleEntity) => void;
-  onLoadMore?: () => void;
 };
 
-export function RulesList({
-  rules,
-  isLoading,
-  onRulePress,
-  onLoadMore,
-}: RulesListProps) {
+export function RulesList({ rules, isLoading, onRulePress }: RulesListProps) {
   const { t } = useTranslation();
 
   if (isLoading && rules.length === 0) {
@@ -65,32 +59,27 @@ export function RulesList({
     );
   }
 
-  const handleScroll = (event: UIEvent<HTMLDivElement>) => {
-    if (!onLoadMore) return;
-
-    const { scrollTop, scrollHeight, clientHeight } = event.currentTarget;
-    if (scrollHeight - scrollTop <= clientHeight * 1.5) {
-      onLoadMore();
-    }
-  };
-
   return (
-    <View
-      style={{ flex: 1, paddingBottom: MOBILE_NAV_HEIGHT, overflow: 'auto' }}
-      onScroll={handleScroll}
-    >
-      {rules.map(rule => (
-        <RulesListItem
-          key={rule.id}
-          rule={rule}
-          onPress={() => onRulePress(rule)}
-        />
-      ))}
+    <View style={{ flex: 1 }}>
+      <GridList
+        aria-label={t('Rules')}
+        aria-busy={isLoading || undefined}
+        items={rules}
+        style={{
+          flex: 1,
+          paddingBottom: MOBILE_NAV_HEIGHT,
+          overflow: 'auto',
+        }}
+      >
+        {rule => (
+          <RulesListItem value={rule} onAction={() => onRulePress(rule)} />
+        )}
+      </GridList>
       {isLoading && (
         <View
           style={{
             alignItems: 'center',
-            paddingVertical: 20,
+            paddingTop: 20,
           }}
         >
           <AnimatedLoading style={{ width: 20, height: 20 }} />

packages/desktop-client/src/components/mobile/rules/RulesListItem.tsx

@@ -1,24 +1,26 @@
 import React from 'react';
+import { GridListItem, type GridListItemProps } from 'react-aria-components';
 import { useTranslation } from 'react-i18next';
 
-import { Button } from '@actual-app/components/button';
+import { SpaceBetween } from '@actual-app/components/space-between';
+import { styles } from '@actual-app/components/styles';
 import { theme } from '@actual-app/components/theme';
 import { View } from '@actual-app/components/view';
 
 import { type RuleEntity } from 'loot-core/types/models';
+import { type WithRequired } from 'loot-core/types/util';
 
 import { ActionExpression } from '@desktop-client/components/rules/ActionExpression';
 import { ConditionExpression } from '@desktop-client/components/rules/ConditionExpression';
 import { groupActionsBySplitIndex } from '@desktop-client/util/ruleUtils';
 
-const ROW_HEIGHT = 60;
+type RulesListItemProps = WithRequired<GridListItemProps<RuleEntity>, 'value'>;
 
-type RulesListItemProps = {
-  rule: RuleEntity;
-  onPress: () => void;
-};
-
-export function RulesListItem({ rule, onPress }: RulesListItemProps) {
+export function RulesListItem({
+  value: rule,
+  style,
+  ...props
+}: RulesListItemProps) {
   const { t } = useTranslation();
 
   // Group actions by splitIndex to handle split transactions
@@ -26,172 +28,160 @@ export function RulesListItem({ rule, onPress }: RulesListItemProps) {
   const hasSplits = actionSplits.length > 1;
 
   return (
-    <Button
-      variant="bare"
-      style={{
-        minHeight: ROW_HEIGHT,
-        width: '100%',
-        borderRadius: 0,
-        borderWidth: '0 0 1px 0',
-        borderColor: theme.tableBorder,
-        borderStyle: 'solid',
-        backgroundColor: theme.tableBackground,
-        flexDirection: 'row',
-        alignItems: 'flex-start',
-        justifyContent: 'flex-start',
-        padding: '8px 16px',
-        gap: 12,
-      }}
-      onPress={onPress}
+    <GridListItem
+      id={rule.id}
+      value={rule}
+      textValue={t('Rule {{id}}', { id: rule.id })}
+      style={{ ...styles.mobileListItem, padding: '8px 16px', ...style }}
+      {...props}
     >
-      {/* Column 1: PRE/POST pill */}
-      <View
-        style={{
-          flexShrink: 0,
-          paddingTop: 2, // Slight top padding to align with text baseline
-        }}
-      >
+      <SpaceBetween gap={12} style={{ alignItems: 'flex-start' }}>
+        {/* Column 1: PRE/POST pill */}
         <View
           style={{
-            backgroundColor:
-              rule.stage === 'pre'
-                ? theme.noticeBackgroundLight
-                : rule.stage === 'post'
-                  ? theme.warningBackground
-                  : theme.pillBackgroundSelected,
-            paddingLeft: 6,
-            paddingRight: 6,
-            paddingTop: 2,
-            paddingBottom: 2,
-            borderRadius: 3,
+            flexShrink: 0,
+            paddingTop: 2, // Slight top padding to align with text baseline
           }}
         >
-          <span
+          <View
             style={{
-              fontSize: 11,
-              fontWeight: 500,
-              color:
+              backgroundColor:
                 rule.stage === 'pre'
-                  ? theme.noticeTextLight
+                  ? theme.noticeBackgroundLight
                   : rule.stage === 'post'
-                    ? theme.warningText
-                    : theme.pillTextSelected,
+                    ? theme.warningBackground
+                    : theme.pillBackgroundSelected,
+              paddingLeft: 6,
+              paddingRight: 6,
+              paddingTop: 2,
+              paddingBottom: 2,
+              borderRadius: 3,
             }}
           >
-            {rule.stage === 'pre'
-              ? t('PRE')
-              : rule.stage === 'post'
-                ? t('POST')
-                : t('DEFAULT')}
-          </span>
-        </View>
-      </View>
-
-      {/* Column 2: IF and THEN blocks */}
-      <View
-        style={{
-          flex: 1,
-          flexDirection: 'column',
-          gap: 4,
-        }}
-      >
-        {/* IF conditions block */}
-        <View
-          style={{
-            flexDirection: 'row',
-            alignItems: 'center',
-            flexWrap: 'wrap',
-            gap: 6,
-          }}
-        >
-          <span
-            style={{
-              fontSize: 13,
-              fontWeight: 600,
-              color: theme.pageTextLight,
-              marginRight: 4,
-            }}
-          >
-            {t('IF')}
-          </span>
-
-          {rule.conditions.map((condition, index) => (
-            <View key={index} style={{ marginRight: 4, marginBottom: 2 }}>
-              <ConditionExpression
-                field={condition.field}
-                op={condition.op}
-                value={condition.value}
-                options={condition.options}
-                inline={true}
-              />
-            </View>
-          ))}
+            <span
+              style={{
+                fontSize: 11,
+                fontWeight: 500,
+                color:
+                  rule.stage === 'pre'
+                    ? theme.noticeTextLight
+                    : rule.stage === 'post'
+                      ? theme.warningText
+                      : theme.pillTextSelected,
+              }}
+              data-testid="rule-stage-badge"
+            >
+              {rule.stage === 'pre'
+                ? t('PRE')
+                : rule.stage === 'post'
+                  ? t('POST')
+                  : t('DEFAULT')}
+            </span>
+          </View>
         </View>
 
-        {/* THEN actions block */}
+        {/* Column 2: IF and THEN blocks */}
         <View
           style={{
+            flex: 1,
             flexDirection: 'column',
-            alignItems: 'flex-start',
             gap: 4,
           }}
         >
-          <span
+          {/* IF conditions block */}
+          <SpaceBetween gap={6}>
+            <span
+              style={{
+                fontSize: 13,
+                fontWeight: 600,
+                color: theme.pageTextLight,
+                marginRight: 4,
+              }}
+            >
+              {t('IF')}
+            </span>
+
+            {rule.conditions.map((condition, index) => (
+              <View key={index} style={{ marginRight: 4, marginBottom: 2 }}>
+                <ConditionExpression
+                  field={condition.field}
+                  op={condition.op}
+                  value={condition.value}
+                  options={condition.options}
+                  inline={true}
+                />
+              </View>
+            ))}
+          </SpaceBetween>
+
+          {/* THEN actions block */}
+          <View
             style={{
-              fontSize: 13,
-              fontWeight: 600,
-              color: theme.pageTextLight,
-              marginBottom: 2,
+              flexDirection: 'column',
+              alignItems: 'flex-start',
+              gap: 4,
             }}
           >
-            {t('THEN')}
-          </span>
+            <span
+              style={{
+                fontSize: 13,
+                fontWeight: 600,
+                color: theme.pageTextLight,
+                marginBottom: 2,
+              }}
+            >
+              {t('THEN')}
+            </span>
 
-          {hasSplits
-            ? actionSplits.map((split, i) => (
-                <View
-                  key={split.id}
-                  style={{
-                    width: '100%',
-                    flexDirection: 'column',
-                    alignItems: 'flex-start',
-                    marginTop: i > 0 ? 4 : 0,
-                    padding: '6px',
-                    borderColor: theme.tableBorder,
-                    borderWidth: '1px',
-                    borderRadius: '5px',
-                  }}
-                >
-                  <span
+            {hasSplits
+              ? actionSplits.map((split, i) => (
+                  <View
+                    key={i}
                     style={{
-                      fontSize: 11,
-                      fontWeight: 500,
-                      color: theme.pageTextLight,
-                      marginBottom: 4,
+                      width: '100%',
+                      flexDirection: 'column',
+                      alignItems: 'flex-start',
+                      marginTop: i > 0 ? 4 : 0,
+                      padding: '6px',
+                      borderColor: theme.tableBorder,
+                      borderWidth: '1px',
+                      borderRadius: '5px',
                     }}
                   >
-                    {i ? t('Split {{num}}', { num: i }) : t('Apply to all')}
-                  </span>
-                  {split.actions.map((action, j) => (
-                    <View
-                      key={j}
+                    <span
                       style={{
-                        marginBottom: j !== split.actions.length - 1 ? 2 : 0,
-                        maxWidth: '100%',
+                        fontSize: 11,
+                        fontWeight: 500,
+                        color: theme.pageTextLight,
+                        marginBottom: 4,
                       }}
                     >
-                      <ActionExpression {...action} />
-                    </View>
-                  ))}
-                </View>
-              ))
-            : rule.actions.map((action, index) => (
-                <View key={index} style={{ marginBottom: 2, maxWidth: '100%' }}>
-                  <ActionExpression {...action} />
-                </View>
-              ))}
+                      {i ? t('Split {{num}}', { num: i }) : t('Apply to all')}
+                    </span>
+                    {split.actions.map((action, j) => (
+                      <View
+                        key={j}
+                        style={{
+                          marginBottom: j !== split.actions.length - 1 ? 2 : 0,
+                          maxWidth: '100%',
+                        }}
+                      >
+                        <ActionExpression {...action} />
+                      </View>
+                    ))}
+                  </View>
+                ))
+              : rule.actions.map((action, index) => (
+                  <View
+                    key={index}
+                    style={{ marginBottom: 2, maxWidth: '100%' }}
+                  >
+                    <ActionExpression {...action} />
+                  </View>
+                ))}
+          </View>
         </View>
-      </View>
-    </Button>
+      </SpaceBetween>
+    </GridListItem>
   );
 }

upcoming-release-notes/5804.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [MatissJanis]
+---
+
+Mobile rules - refactor to use react-aria GridList