Refact: Expense Speedrun Pt. 3 (#10674)

* fix: remove data.expense mutation in favor of proper payee fragment * fix: ExpenseList shortcut navigation redrawing bug * refact: integrate attachments and ExpenseDrawer * fix: attachment glitch when ExpenseDrawer is animating in
opencollective · Oct 1, 2024 · d163638 · d163638
1 parent 1d11d9b
commit d163638
Show file tree

Hide file tree

Showing 12 changed files with 146 additions and 107 deletions.
diff --git a/components/FilesViewerModal.tsx b/components/FilesViewerModal.tsx
@@ -134,6 +134,7 @@ type FilesViewerModalProps = {
   allowOutsideInteraction?: boolean;
   canDownload?: boolean;
   canOpenInNewWindow?: boolean;
+  hideCloseButton?: boolean;
 };
 
 export default function FilesViewerModal({
@@ -144,6 +145,7 @@ export default function FilesViewerModal({
   openFileUrl,
   canDownload = true,
   canOpenInNewWindow = true,
+  hideCloseButton = false,
 }: FilesViewerModalProps) {
   const intl = useIntl();
   const initialIndex = openFileUrl ? files?.findIndex(f => f.url === openFileUrl) : 0;
@@ -217,7 +219,7 @@ export default function FilesViewerModal({
           // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
           <div
             className={clsx(
-              'absolute inset-0 z-[1500] bg-foreground/25 backdrop-blur-sm',
+              'absolute inset-0 z-[1500] backdrop-blur-sm',
               open ? 'animate-in fade-in-0' : 'animate-out fade-out-0',
             )}
             onClick={onClose}
@@ -279,16 +281,18 @@ export default function FilesViewerModal({
                 </ButtonLink>
               </StyledTooltip>
             )}
-            <StyledTooltip
-              containerCursor="pointer"
-              noArrow
-              content={intl.formatMessage({ id: 'Close', defaultMessage: 'Close' })}
-              delayHide={0}
-            >
-              <Button onClick={onClose}>
-                <X size="24" aria-hidden="true" />
-              </Button>
-            </StyledTooltip>
+            {!hideCloseButton && (
+              <StyledTooltip
+                containerCursor="pointer"
+                noArrow
+                content={intl.formatMessage({ id: 'Close', defaultMessage: 'Close' })}
+                delayHide={0}
+              >
+                <Button onClick={onClose}>
+                  <X size="24" aria-hidden="true" />
+                </Button>
+              </StyledTooltip>
+            )}
           </Flex>
         </Header>
         {hasMultipleFiles && (

diff --git a/components/budget/ExpenseBudgetItem.js b/components/budget/ExpenseBudgetItem.js
@@ -444,7 +444,7 @@ const ExpenseBudgetItem = ({
                       fontSize="11px"
                       cursor="pointer"
                       buttonSize="tiny"
-                      onClick={() => setShowFilesViewerModal(true)}
+                      onClick={useDrawer ? expandExpense : () => setShowFilesViewerModal(true)}
                       px={2}
                       ml={-2}
                       isBorderless

diff --git a/components/expenses/Expense.tsx b/components/expenses/Expense.tsx
@@ -18,9 +18,10 @@ import { formatErrorMessage, getErrorFromGraphqlException } from '../../lib/erro
 import { getFilesFromExpense, getPayoutProfiles } from '../../lib/expenses';
 import { API_V2_CONTEXT } from '../../lib/graphql/helpers';
 import { ExpenseStatus } from '../../lib/graphql/types/v2/graphql';
-import useKeyboardKey, { E } from '../../lib/hooks/useKeyboardKey';
+import useKeyboardKey, { E, ESCAPE_KEY } from '../../lib/hooks/useKeyboardKey';
 import useLoggedInUser from '../../lib/hooks/useLoggedInUser';
 import { usePrevious } from '../../lib/hooks/usePrevious';
+import { useWindowResize, VIEWPORTS } from '../../lib/hooks/useWindowResize';
 import { itemHasOCR } from './lib/ocr';
 
 import ConfirmationModal from '../ConfirmationModal';
@@ -43,6 +44,7 @@ import StyledButton from '../StyledButton';
 import StyledCheckbox from '../StyledCheckbox';
 import StyledLink from '../StyledLink';
 import { H1, H5, Span } from '../Text';
+import { DialogPortal } from '../ui/Dialog';
 
 import { editExpenseMutation } from './graphql/mutations';
 import { expensePageQuery } from './graphql/queries';
@@ -116,6 +118,7 @@ function Expense(props) {
     legacyExpenseId,
     isDrawer,
     enableKeyboardShortcuts,
+    onClose,
   } = props;
   const { LoggedInUser, loadingLoggedInUser } = useLoggedInUser();
   const intl = useIntl();
@@ -222,17 +225,26 @@ function Expense(props) {
     },
   });
 
+  useKeyboardKey({
+    keyMatch: ESCAPE_KEY,
+    callback: e => {
+      if (props.isDrawer && state.status !== PAGE_STATUS.EDIT) {
+        e.preventDefault();
+        onClose?.();
+      }
+    },
+  });
+
   const [editExpense] = useMutation(editExpenseMutation, {
     context: API_V2_CONTEXT,
   });
 
   const expenseTopRef = useRef(null);
   const { status, editedExpense } = state;
+  const { viewport } = useWindowResize(null, { useMinWidth: true });
+  const isDesktop = viewport === VIEWPORTS.LARGE;
 
-  const expense = cloneDeep(data?.expense);
-  if (expense && data?.expensePayeeStats?.payee?.stats) {
-    expense.payee.stats = data.expensePayeeStats?.payee?.stats;
-  }
+  const expense = data?.expense;
   const loggedInAccount = data?.loggedInAccount;
   const collective = expense?.account;
   const host = expense?.host ?? collective?.host;
@@ -397,6 +409,11 @@ function Expense(props) {
 
   const files = React.useMemo(() => getFilesFromExpense(expense, intl), [expense]);
 
+  useEffect(() => {
+    const showFilesViewerModal = isDrawer && isDesktop && files?.length > 0;
+    setState(state => ({ ...state, showFilesViewerModal }));
+  }, [files, isDesktop, isDrawer]);
+
   const confirmSaveButtons = (
     <Flex flex={1} flexWrap="wrap" gridGap={[2, 3]}>
       <StyledButton
@@ -776,19 +793,24 @@ function Expense(props) {
       )}
 
       {state.showFilesViewerModal && (
-        <FilesViewerModal
-          allowOutsideInteraction={isDrawer}
-          files={files}
-          parentTitle={intl.formatMessage(
-            {
-              defaultMessage: 'Expense #{expenseId} attachment',
-              id: 'At2m8o',
-            },
-            { expenseId: expense.legacyId },
-          )}
-          openFileUrl={openUrl}
-          onClose={() => setState(state => ({ ...state, showFilesViewerModal: false }))}
-        />
+        <DialogPortal>
+          <FilesViewerModal
+            allowOutsideInteraction={isDrawer}
+            files={files}
+            parentTitle={intl.formatMessage(
+              {
+                defaultMessage: 'Expense #{expenseId} attachment',
+                id: 'At2m8o',
+              },
+              { expenseId: expense.legacyId },
+            )}
+            openFileUrl={openUrl}
+            onClose={
+              isDrawer && isDesktop ? onClose : () => setState(state => ({ ...state, showFilesViewerModal: false }))
+            }
+            hideCloseButton={isDrawer && isDesktop}
+          />
+        </DialogPortal>
       )}
     </Box>
   );
@@ -808,6 +830,7 @@ Expense.propTypes = {
   fetchMore: PropTypes.func,
   startPolling: PropTypes.func,
   stopPolling: PropTypes.func,
+  onClose: PropTypes.func,
   isRefetchingDataForUser: PropTypes.bool,
   drawerActionsContainer: PropTypes.object,
   isDrawer: PropTypes.bool,

diff --git a/components/expenses/ExpenseDrawer.tsx b/components/expenses/ExpenseDrawer.tsx
@@ -58,6 +58,7 @@ export default function ExpenseDrawer({ openExpenseLegacyId, handleClose, initia
         startPolling={startPolling}
         stopPolling={stopPolling}
         isDrawer
+        onClose={handleClose}
         enableKeyboardShortcuts={hasKeyboardShortcutsEnabled}
       />
     </Drawer>

diff --git a/components/expenses/ExpenseSummaryAdditionalInformation.js b/components/expenses/ExpenseSummaryAdditionalInformation.js
@@ -271,7 +271,7 @@ const ExpenseSummaryAdditionalInformation = ({
                     ? VIRTUAL_CARD
                     : expense.payoutMethod?.type
               }
-              name={expense?.virtualCard?.name && `${expense.virtualCard.name} Card (${expense.virtualCard.last4})`}
+              name={expense.virtualCard?.name && `${expense.virtualCard.name} Card (${expense.virtualCard.last4})`}
             />
           </Box>
           <Container data-cy="expense-summary-payout-method-data" wordBreak="break-word">

diff --git a/components/expenses/ExpensesList.js b/components/expenses/ExpensesList.js
@@ -97,7 +97,7 @@ const ExpensesList = ({
 
   const [selectedExpenseIndex, setSelectedExpenseIndex] = React.useState();
   const navigateIndex = dif => event => {
-    if (hasKeyboardShortcutsEnabled) {
+    if (hasKeyboardShortcutsEnabled && !openExpenseLegacyId) {
       event.preventDefault();
       let nextIndex = (selectedExpenseIndex ?? -1) + dif;
       if (nextIndex < 0) {

diff --git a/components/expenses/graphql/fragments.ts b/components/expenses/graphql/fragments.ts
@@ -194,6 +194,58 @@ export const expenseValuesByRoleFragment = gql`
   ${accountingCategoryFields}
 `;
 
+export const expensePayeeFieldsFragment = gql`
+  fragment ExpensePayeeFields on Account {
+    id
+    slug
+    name
+    legalName
+    imageUrl
+    type
+    isAdmin
+    isActive
+    description
+    ...AccountHoverCardFields
+    location {
+      id
+      address
+      country
+    }
+    payoutMethods {
+      id
+      type
+      name
+      data
+      isSaved
+    }
+
+    # For Collectives, Funds, Events and Projects
+    ... on AccountWithHost {
+      isApproved
+      host {
+        id
+        slug
+        # For Expenses across hosts
+        payoutMethods {
+          id
+          type
+          name
+          data
+          isSaved
+        }
+      }
+    }
+
+    # For Fiscal Hosts
+    ... on Organization {
+      host {
+        id
+        slug
+      }
+    }
+  }
+`;
+
 export const expensePageExpenseFieldsFragment = gql`
   fragment ExpensePageExpenseFields on Expense {
     id
@@ -292,52 +344,7 @@ export const expensePageExpenseFieldsFragment = gql`
     }
     payee {
       id
-      slug
-      name
-      legalName
-      imageUrl
-      type
-      isAdmin
-      isActive
-      description
-      ...AccountHoverCardFields
-      location {
-        id
-        address
-        country
-      }
-      payoutMethods {
-        id
-        type
-        name
-        data
-        isSaved
-      }
-
-      # For Collectives, Funds, Events and Projects
-      ... on AccountWithHost {
-        isApproved
-        host {
-          id
-          slug
-          # For Expenses across hosts
-          payoutMethods {
-            id
-            type
-            name
-            data
-            isSaved
-          }
-        }
-      }
-
-      # For Fiscal Hosts
-      ... on Organization {
-        host {
-          id
-          slug
-        }
-      }
+      ...ExpensePayeeFields
     }
     payeeLocation {
       id
@@ -617,6 +624,7 @@ export const expensePageExpenseFieldsFragment = gql`
   ${accountingCategoryFields}
   ${accountHoverCardFields}
   ${expenseValuesByRoleFragment}
+  ${expensePayeeFieldsFragment}
 `;
 
 export const expensesListFieldsFragment = gql`

diff --git a/components/expenses/graphql/queries.ts b/components/expenses/graphql/queries.ts
@@ -2,30 +2,20 @@ import { gql } from '../../../lib/graphql/helpers';
 
 import { commentFieldsFragment } from '../../conversations/graphql';
 
-import { expensePageExpenseFieldsFragment, loggedInAccountExpensePayoutFieldsFragment } from './fragments';
+import {
+  expensePageExpenseFieldsFragment,
+  expensePayeeFieldsFragment,
+  loggedInAccountExpensePayoutFieldsFragment,
+} from './fragments';
 
 export const expensePageQuery = gql`
   query ExpensePage($legacyExpenseId: Int!, $draftKey: String, $offset: Int, $totalPaidExpensesDateFrom: DateTime) {
     expense(expense: { legacyId: $legacyExpenseId }, draftKey: $draftKey) {
       id
       ...ExpensePageExpenseFields
-      permissions {
-        canDeclineExpenseInvite(draftKey: $draftKey)
-      }
-      comments(limit: 100, offset: $offset) {
-        totalCount
-        nodes {
-          id
-          ...CommentFields
-        }
-      }
-    }
-
-    # As it uses a dedicated variable this needs to be separated from the ExpensePageExpenseFields fragment
-    expensePayeeStats: expense(expense: { legacyId: $legacyExpenseId }) {
-      id
       payee {
         id
+        ...ExpensePayeeFields
         stats {
           id
           totalPaidExpenses(dateFrom: $totalPaidExpensesDateFrom) {
@@ -46,6 +36,16 @@ export const expensePageQuery = gql`
           }
         }
       }
+      permissions {
+        canDeclineExpenseInvite(draftKey: $draftKey)
+      }
+      comments(limit: 100, offset: $offset) {
+        totalCount
+        nodes {
+          id
+          ...CommentFields
+        }
+      }
     }
 
     loggedInAccount {
@@ -57,4 +57,5 @@ export const expensePageQuery = gql`
   ${loggedInAccountExpensePayoutFieldsFragment}
   ${expensePageExpenseFieldsFragment}
   ${commentFieldsFragment}
+  ${expensePayeeFieldsFragment}
 `;