From ce431eaf7506d449216aa776cd34b630cda21034 Mon Sep 17 00:00:00 2001 From: Andrew Gable Date: Tue, 20 Oct 2026 15:34:37 -0600 Subject: [PATCH] Revert "Restore: Show up to three report fields per row and edit them inline" --- config/eslint/eslint.seatbelt.tsv | 5 +- src/CONST/index.ts | 7 - src/components/DatePicker/index.tsx | 12 +- src/components/DatePicker/types.ts | 9 - .../MoneyRequestReportActionsList.tsx | 3 +- .../MoneyRequestReportEmptyStateView.tsx | 7 +- .../MoneyRequestViewReportFields.tsx | 163 ++--- .../ReportFieldInlineInput.tsx | 305 --------- .../ReportActionItem/MoneyReportView.tsx | 85 ++- .../FilterDropdowns/FilterPopupButton.tsx | 6 +- src/hooks/useSaveReportField.ts | 81 --- src/languages/de.ts | 1 - src/languages/el.ts | 1 - src/languages/en.ts | 1 - src/languages/es.ts | 1 - src/languages/fr.ts | 1 - src/languages/it.ts | 1 - src/languages/ja.ts | 1 - src/languages/nl.ts | 1 - src/languages/pl.ts | 1 - src/languages/pt-BR.ts | 1 - src/languages/zh-hans.ts | 1 - src/libs/ReportActionsUtils.ts | 3 - src/libs/ReportFieldOptionsListUtils.ts | 14 +- src/libs/actions/Report/index.ts | 2 +- src/pages/DynamicEditReportFieldPage.tsx | 43 +- src/pages/EditReportFieldDropdown.tsx | 51 +- src/styles/index.ts | 28 - tests/ui/EditReportFieldDropdownTest.tsx | 131 ---- tests/ui/MoneyReportViewTest.tsx | 48 +- tests/ui/MoneyRequestViewReportFieldsTest.tsx | 601 ------------------ tests/unit/ReportActionsUtilsTest.ts | 48 -- tests/unit/ReportFieldOptionsListUtilsTest.ts | 73 +-- 33 files changed, 170 insertions(+), 1566 deletions(-) delete mode 100644 src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx delete mode 100644 src/hooks/useSaveReportField.ts delete mode 100644 tests/ui/EditReportFieldDropdownTest.tsx delete mode 100644 tests/ui/MoneyRequestViewReportFieldsTest.tsx diff --git a/config/eslint/eslint.seatbelt.tsv b/config/eslint/eslint.seatbelt.tsv index 59591527b9c6..9d75e115f40c 100644 --- a/config/eslint/eslint.seatbelt.tsv +++ b/config/eslint/eslint.seatbelt.tsv @@ -243,6 +243,7 @@ "../../src/components/MoneyRequestConfirmationListFooter/sections/ReceiptSection.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../src/components/MoneyRequestHeader.tsx" "no-restricted-imports" 1 "../../src/components/MoneyRequestReportView/MoneyRequestReportTransactionsNavigation.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 +"../../src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx" "@typescript-eslint/no-deprecated/MenuItemWithTopDescription" 1 "../../src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../src/components/MultifactorAuthentication/components/Modals/createCancelConfirmModal.tsx" "@typescript-eslint/no-deprecated/ConfirmModal" 1 "../../src/components/MultifactorAuthentication/components/OutcomeScreen/OutcomeScreenBase.tsx" "no-restricted-imports" 1 @@ -308,7 +309,8 @@ "../../src/components/ReceiptHoverZoom/useReceiptHoverZoom.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../src/components/ReimbursementAccountLoadingIndicator.tsx" "no-restricted-imports" 1 "../../src/components/ReportActionAvatars/useReportPreviewSenderID.ts" "@typescript-eslint/no-unsafe-type-assertion" 2 -"../../src/components/ReportActionItem/MoneyReportView.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 +"../../src/components/ReportActionItem/MoneyReportView.tsx" "@typescript-eslint/no-deprecated/MenuItemWithTopDescription" 1 +"../../src/components/ReportActionItem/MoneyReportView.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 "../../src/components/ReportActionItem/MoneyRequestReceiptView.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 "../../src/components/ReportActionItem/MoneyRequestReceiptView.tsx" "react-hooks/set-state-in-effect" 1 "../../src/components/ReportActionItem/MoneyRequestView.tsx" "@typescript-eslint/no-deprecated/MenuItemWithTopDescription" 5 @@ -955,6 +957,7 @@ "../../src/pages/domain/Members/DomainMembersPage.tsx" "no-restricted-imports" 1 "../../src/pages/domain/Members/MoveUserBetweenGroupsPage.tsx" "no-restricted-imports" 1 "../../src/pages/domain/Members/MoveUsersBetweenGroupsPage.tsx" "no-restricted-imports" 1 +"../../src/pages/DynamicEditReportFieldPage.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../src/pages/DynamicEditReportFieldPage.tsx" "no-restricted-imports" 1 "../../src/pages/DynamicFlagCommentPage.tsx" "@typescript-eslint/no-deprecated/MenuItem" 1 "../../src/pages/DynamicFlagCommentPage.tsx" "no-restricted-imports" 1 diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 1fe766a39e12..4ed4d1f8f356 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -7707,12 +7707,6 @@ const CONST = { REPORT_FIELD_TITLE_FIELD_ID: 'text_title', - /** How many report fields are shown side by side in the report view on a wide layout */ - REPORT_FIELDS_PER_ROW: 3, - - /** Below this many options, a report field list is short enough to scan without a search input */ - REPORT_FIELD_LIST_SEARCH_THRESHOLD: 8, - MOBILE_PAGINATION_SIZE: 15, WEB_PAGINATION_SIZE: 30, @@ -10064,7 +10058,6 @@ const CONST = { MODERATION_BUTTON: 'Report-ModerationButton', MONEY_REQUEST_REPORT_ACTIONS_LIST_SELECT_ALL: 'MoneyRequestReportActionsList-SelectAll', MONEY_REQUEST_REPORT_TRANSACTION_ITEM: 'MoneyRequestReportTransactionItem', - REPORT_FIELD_DELETE_BUTTON: 'Report-ReportFieldDeleteButton', REPORT_ACTION_AVATAR: 'Report-ReportActionAvatar', PARTICIPANTS_ROW: 'Report-ParticipantsRow', ROOM_MEMBERS_ROW: 'Report-RoomMembersRow', diff --git a/src/components/DatePicker/index.tsx b/src/components/DatePicker/index.tsx index bed4bf8fd42f..58d4d0f13fcd 100644 --- a/src/components/DatePicker/index.tsx +++ b/src/components/DatePicker/index.tsx @@ -49,13 +49,10 @@ function DatePicker({ forwardedFSClass, shouldDeferShowUntilPositioned = false, shouldDismissKeyboardBeforeShow = false, - wrapperStyle, rightHandSideComponent, onPickerVisibilityChange, shouldHideCalendarIcon = false, icon, - shouldEnableMonthYearBackdropInNarrowPane = false, - textInputContainerStyles, }: DateInputWithPickerProps) { const icons = useMemoizedLazyExpensifyIcons(['Calendar']); const styles = useThemeStyles(); @@ -228,15 +225,13 @@ function DatePicker({ <> showDatePickerModal()} onSubmitEditing={() => showDatePickerModal()} onKeyPress={handleInputKeyPress} - textInputContainerStyles={[textInputContainerStyles, isModalVisible && styles.borderColorFocus]} + textInputContainerStyles={isModalVisible ? styles.borderColorFocus : {}} shouldHideClearButton={shouldHideClearButton} onClearInput={handleClear} clearButtonStyle={[styles.mh0, {paddingHorizontal: iconPaddingHorizontal}]} @@ -275,7 +270,6 @@ function DatePicker({ shouldPositionFromTop={!isInverted} forwardedFSClass={forwardedFSClass} shouldCloseWhenBrowserNavigationChanged - shouldEnableMonthYearBackdropInNarrowPane={shouldEnableMonthYearBackdropInNarrowPane} /> ); diff --git a/src/components/DatePicker/types.ts b/src/components/DatePicker/types.ts index d8170c4a43af..af2242bc12cb 100644 --- a/src/components/DatePicker/types.ts +++ b/src/components/DatePicker/types.ts @@ -5,8 +5,6 @@ import type {ForwardedFSClassProps} from '@libs/Fullstory/types'; import type {OnyxFormValuesMapping} from '@src/ONYXKEYS'; -import type {StyleProp, ViewStyle} from 'react-native'; - type DatePickerBaseProps = ForwardedFSClassProps & { /** The selected date as a `yyyy-MM-dd` string, read with DateUtils.parseCalendarDate. `onInputChange` is called with the picked date in the same format. */ value?: string; @@ -66,13 +64,6 @@ type DateInputWithPickerProps = DatePickerBaseProps & */ shouldDismissKeyboardBeforeShow?: boolean; - /** - * Style for the view that wraps the input and anchors the calendar. It carries a vertical margin by default, - * which a caller laying the field out itself can override, for example to line the input up with a plain - * `TextInput` beside it. - */ - wrapperStyle?: StyleProp; - /** * Reports whether the calendar is open. Opening the picker blurs the input, so this is the signal for "the * user is on this field" rather than `onFocus`, and it is what drives the input's focused border. diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx index 3a90fc84dbc6..a2b475766e8e 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -358,8 +358,7 @@ function MoneyRequestReportActionsListContent({reportIDFromRoute, onLayout}: Mon onViewableItemsChanged={onViewableItemsChanged} onEndReached={onEndReached} onStartReached={onStartReached} - // 20px between the report header and the first row of content, which is the report field inputs when the report has fields. - contentContainerStyle={[styles.pt5, shouldReserveBulkActionBarSpace && styles.bulkActionBarListSpacing]} + contentContainerStyle={[shouldUseNarrowLayout ? styles.pt4 : styles.pt3, shouldReserveBulkActionBarSpace && styles.bulkActionBarListSpacing]} isLoadingInitialActions={isInitialReportLoadPending} /* This list is not inverted, so the footer is the bottom of the message feed — the same position the indicator occupies in the inverted ReportActionsList. */ diff --git a/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx b/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx index 3892322c0d79..729a87232a79 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx @@ -27,12 +27,7 @@ function MoneyRequestReportEmptyStateView({report, policy, onLayout}: MoneyReque const styles = useThemeStyles(); return ( - // `pt5` matches the 20px the unified list puts between the report header and its first row. - // `handled` lets a tap on another report field or a dropdown's clear button land while the keyboard is open, matching the unified list. - + ; @@ -47,99 +44,53 @@ type MoneyRequestViewReportFieldsProps = { /** Indicates whether we have any pending actions from parent component */ pendingAction?: PendingAction; - - /** Extra styles for the block. Each call site sits in a container with its own padding, so it sets the spacing it needs. */ - style?: StyleProp; - - /** - * Stacks the fields one per row at any width, instead of filling rows of `CONST.REPORT_FIELDS_PER_ROW`. - * A one-expense report reads as a single expense rather than a table, so its fields stay vertical there. - * @default false - */ - shouldUseSingleColumn?: boolean; }; type EnrichedPolicyReportField = { fieldValue: string; isFieldDisabled: boolean; - - /** Whether the field was deleted from the workspace but still holds a value on this report, so it can be removed */ - isFieldDeletable: boolean; fieldKey: string; violation: ReportViolationName | undefined; violationTranslation: string; } & PolicyReportField; -function ReportFieldView( - reportField: EnrichedPolicyReportField, - report: OnyxEntry, - policy: OnyxEntry, - styles: ThemeStyles, - onSaveValue: (reportField: PolicyReportField, value: string) => void, - onDelete: (reportField: EnrichedPolicyReportField) => Promise, - pendingAction?: PendingAction, -) { +function ReportFieldView(reportField: EnrichedPolicyReportField, report: OnyxEntry, styles: ThemeStyles, pendingAction?: PendingAction) { return ( - clearReportFieldKeyErrors(report?.reportID, reportField.fieldKey)} > - clearReportFieldKeyErrors(report?.reportID, reportField.fieldKey)} - > - onSaveValue(reportField, value)} - onDelete={ - reportField.isFieldDeletable - ? () => { - onDelete(reportField); - } - : undefined + { + if (!report?.policyID) { + return; } - /> - - + + Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.EDIT_REPORT_FIELD.getRoute(report.policyID, reportField.fieldID))); + }} + shouldShowRightIcon={!reportField.isFieldDisabled} + wrapperStyle={[styles.pv2, styles.taskDescriptionMenuItem]} + shouldGreyOutWhenDisabled={false} + numberOfLinesTitle={0} + interactive={!reportField.isFieldDisabled} + titleWithTooltips={[]} + brickRoadIndicator={reportField.violation ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} + errorText={reportField.violationTranslation} + /> + ); } -function MoneyRequestViewReportFields({report, policy, pendingAction, style, shouldUseSingleColumn = false}: MoneyRequestViewReportFieldsProps) { +function MoneyRequestViewReportFields({report, policy, pendingAction}: MoneyRequestViewReportFieldsProps) { const styles = useThemeStyles(); - // The report view is a RightModalNavigator screen shown as a wide RHP, where `useResponsiveLayout` reports a narrow layout at any pane width. - const {shouldUseNarrowLayout} = useResponsiveLayoutOnWideRHP(); - const saveReportField = useSaveReportField(report, policy); const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${getNonEmptyStringOnyxID(report?.reportID)}`); const {getCurrencyDecimals} = useCurrencyListActions(); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); - const {translate} = useLocalize(); - const {showConfirmModal} = useConfirmModal(); - - const deleteField = async (reportField: EnrichedPolicyReportField) => { - const result = await showConfirmModal({ - title: translate('workspace.reportFields.delete'), - prompt: translate('workspace.reportFields.deleteConfirmation'), - confirmText: translate('common.delete'), - cancelText: translate('common.cancel'), - buttonVariant: CONST.BUTTON_VARIANT.DANGER, - shouldEnableNewFocusManagement: true, - }); - if (result.action !== ModalActions.CONFIRM || !report?.reportID) { - return; - } - // The failure data restores whatever field is passed here, so it gets the field as stored on the report rather - // than the copy enriched for display. - deleteReportField(report.reportID, report.fieldList?.[reportField.fieldKey] ?? reportField); - }; const sortedPolicyReportFields = useMemo((): EnrichedPolicyReportField[] => { const {fieldValues, fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); @@ -150,19 +101,10 @@ function MoneyRequestViewReportFields({report, policy, pendingAction, style, sho .filter((reportField) => !shouldHideSingleReportField(reportField)) .sort(({orderWeight: firstOrderWeight}, {orderWeight: secondOrderWeight}) => firstOrderWeight - secondOrderWeight) .map((field): EnrichedPolicyReportField => { + const fieldValue = resolveReportFieldValue(field, report, policy, fieldValues, fieldsByName, getCurrencyDecimals); const isFieldDisabled = isReportFieldDisabledForUser(report, field, policy, currentUserAccountID, rules); const isDeletedFormulaField = field.type === CONST.REPORT_FIELD_TYPES.FORMULA && field.deletable; const fieldKey = getReportFieldKey(field.fieldID); - const isFieldOnPolicy = !!(policy?.fieldList?.[fieldKey] ?? policy?.fieldList?.[field.fieldID]); - const isEditable = !isFieldDisabled || isDeletedFormulaField; - - // An input the user can type into is seeded with the raw value, the same as the report field editor page. - // Seeding it with the computed value would save the resolved text over a `{field:X}` default the first - // time it was edited, so the field would stop following the field it refers to. - const shouldUseRawValue = isEditable && field.type !== CONST.REPORT_FIELD_TYPES.FORMULA; - const fieldValue = shouldUseRawValue - ? (field.value ?? field.defaultValue ?? '') - : resolveReportFieldValue(field, report, policy, fieldValues, fieldsByName, getCurrencyDecimals); const violation = isFieldDisabled ? undefined : getFieldViolation(field); const violationTranslation = getFieldViolationTranslation(field, violation); @@ -170,11 +112,7 @@ function MoneyRequestViewReportFields({report, policy, pendingAction, style, sho return { ...field, fieldValue, - isFieldDisabled: !isEditable, - // Only fields that are no longer on the workspace can be deleted. Their `deletable` flag is not - // checked: once the workspace drops a field, whether it was required no longer applies, and the - // report's copy of a date field can still be flagged as not deletable. - isFieldDeletable: field.fieldID !== CONST.REPORT_FIELD_TITLE_FIELD_ID && !isFieldOnPolicy && isEditable, + isFieldDisabled: isFieldDisabled && !isDeletedFormulaField, fieldKey, violation, violationTranslation, @@ -190,35 +128,10 @@ function MoneyRequestViewReportFields({report, policy, pendingAction, style, sho return null; } - const columnCount = shouldUseNarrowLayout || shouldUseSingleColumn ? 1 : CONST.REPORT_FIELDS_PER_ROW; - const fieldRows: EnrichedPolicyReportField[][] = []; - for (let index = 0; index < sortedPolicyReportFields.length; index += columnCount) { - fieldRows.push(sortedPolicyReportFields.slice(index, index + columnCount)); - } - return ( - - {fieldRows.map((fieldRow) => { - const rowKey = `reportFieldRow-${fieldRow.at(0)?.fieldKey}`; - - return ( - - {fieldRow.map((reportField) => ReportFieldView(reportField, report, policy, styles, saveReportField, deleteField, pendingAction))} - {/* A partly filled last row is padded out so its fields stay the same width as the rows above it. */} - {Array.from({length: columnCount - fieldRow.length}, (_unused, index) => ( - - ))} - - ); + + {sortedPolicyReportFields.map((reportField) => { + return ReportFieldView(reportField, report, styles, pendingAction); })} ); diff --git a/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx b/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx deleted file mode 100644 index 33d566ceb5a5..000000000000 --- a/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx +++ /dev/null @@ -1,305 +0,0 @@ -/** - * Renders a single report field in the report view as an editable input instead of a row that navigates to the - * report field editor page. Text and formula fields are typed into directly, dates open the calendar in place, and - * list fields open the option list in a dropdown anchored under the input, so changing a value never takes the user - * off the report. - */ - -import DatePicker from '@components/DatePicker'; -import Icon from '@components/Icon'; -import {PressableWithFeedback} from '@components/Pressable'; -import FilterPopupButton from '@components/Search/FilterDropdowns/FilterPopupButton'; -import type {FilterPopupButtonProps} from '@components/Search/FilterDropdowns/FilterPopupButton'; -import TextInput from '@components/TextInput'; -import type {BaseTextInputRef} from '@components/TextInput/BaseTextInput/types'; - -import useIsInLandscapeMode from '@hooks/useIsInLandscapeMode'; -import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; -import useLocalize from '@hooks/useLocalize'; -import useResponsiveLayout from '@hooks/useResponsiveLayout'; -import useTheme from '@hooks/useTheme'; -import useThemeStyles from '@hooks/useThemeStyles'; -import useWindowDimensions from '@hooks/useWindowDimensions'; - -import {hasCircularReferences} from '@libs/Formula'; -import type {FieldList} from '@libs/Formula'; -import StringUtils from '@libs/StringUtils'; - -import EditReportFieldDropdown from '@pages/EditReportFieldDropdown'; - -import CONST from '@src/CONST'; -import type {PolicyReportField} from '@src/types/onyx'; - -import type {GestureResponderEvent} from 'react-native'; - -import {Str} from 'expensify-common'; -import React, {useRef, useState} from 'react'; -import {View} from 'react-native'; - -type ReportFieldInlineInputProps = { - reportField: PolicyReportField; - - /** Key the field is stored under, used as the input ID */ - fieldKey: string; - - /** Value currently shown for the field */ - value: string; - - /** Whether the current user is allowed to change the value */ - isDisabled: boolean; - - /** Message shown under the input, for example a field violation */ - errorText?: string; - - /** Policy field list, used to reject values that make a formula refer to itself */ - fieldList?: FieldList; - - onSaveValue: (value: string) => void; - - /** - * Removes the field from the report. Passed only for a field that was deleted from the workspace, so the field - * shows a delete button on its right edge. Without it the stale value could never be cleared off the report. - */ - onDelete?: () => void; -}; - -function ReportFieldInlineInput({reportField, fieldKey, value, isDisabled, errorText, fieldList, onSaveValue, onDelete}: ReportFieldInlineInputProps) { - const theme = useTheme(); - const styles = useThemeStyles(); - const {translate} = useLocalize(); - const {windowHeight} = useWindowDimensions(); - const isInLandscapeMode = useIsInLandscapeMode(); - const {isInNarrowPaneModal} = useResponsiveLayout(); - const icons = useMemoizedLazyExpensifyIcons(['DownArrow', 'Trashcan']); - - const [draftValue, setDraftValue] = useState(value); - const [previousValue, setPreviousValue] = useState(value); - const [validationError, setValidationError] = useState(''); - - // Tracks the value that was last sent to the server so a second save attempt for the same value, for example when - // the input is focused and blurred again before the update comes back, is skipped while it is still in flight. - const [lastSavedValue, setLastSavedValue] = useState(value); - - // On Android a tap on the list field's input reports the same press twice, once as `onPress` and once as - // `onPressOut`, and the dropdown trigger toggles. Remembering the last press lets each tap open the list once - // instead of opening and then closing it. - const lastOptionsPressEventRef = useRef(null); - - const textInputRef = useRef(null); - - // The saved value changes while this input is mounted whenever the field is updated elsewhere, for example from - // another device or by an optimistic update, so the draft follows it. - if (value !== previousValue) { - setPreviousValue(value); - setDraftValue(value); - setValidationError(''); - setLastSavedValue(value); - } - - const label = Str.UCFirst(reportField.name); - const isReadOnly = isDisabled || reportField.type === CONST.REPORT_FIELD_TYPES.FORMULA; - - const saveDraftValue = () => { - const draftWithoutLineBreaks = StringUtils.lineBreaksToSpaces(draftValue); - const trimmedValue = draftWithoutLineBreaks.trim(); - - // Whitespace alone is saved as an empty value, so clearing a field never stores the spaces left behind. - const valueToSave = trimmedValue === '' ? '' : draftWithoutLineBreaks; - - // Only a text field can be cleared, the same as on OldDot, so any other field goes back to its saved value. - const canClearField = reportField.type === CONST.REPORT_FIELD_TYPES.TEXT; - - if (trimmedValue === lastSavedValue.trim() || (trimmedValue === '' && !canClearField)) { - setValidationError(''); - // Nothing is sent, so the saved value never changes and the resync above that follows it never runs. - // Without this the field would keep displaying the whitespace the user left behind until it remounted, - // at which point the text would visibly snap back to the value that was really stored. - setDraftValue(lastSavedValue); - return; - } - - if (hasCircularReferences(trimmedValue, reportField.name, fieldList)) { - setValidationError(translate('workspace.reportFields.circularReferenceError')); - return; - } - - setValidationError(''); - setLastSavedValue(valueToSave); - onSaveValue(valueToSave); - }; - - const saveSelectedOption = (selectedValue: string) => { - if (selectedValue === lastSavedValue) { - return; - } - setLastSavedValue(selectedValue); - onSaveValue(selectedValue); - }; - - // The input's right corners are squared off so the delete button can sit flush against it, and the input's own - // right border becomes the divider between the two. - const textInputContainerStyles = onDelete ? styles.noRightBorderRadius : undefined; - - const renderWithDeleteButton = (field: React.ReactNode) => { - if (!onDelete) { - return field; - } - - return ( - // The button keeps the input's height and stays at the top, so an error message under the input grows the - // field downwards without stretching the button. - - {field} - - - - - ); - }; - - if (reportField.type === CONST.REPORT_FIELD_TYPES.DATE && !isReadOnly) { - return renderWithDeleteButton( - saveSelectedOption(selectedDate)} - // The grid puts its own gap between the fields, so the default vertical margin would push a date - // input below the plain text inputs sharing its row. - wrapperStyle={styles.mv0} - shouldDeferShowUntilPositioned - shouldHideClearButton - // The Month/Year pickers are right-docked, and a right-docked modal opened from the RHP drops its - // backdrop by default, so the report behind them would not be dimmed. - shouldEnableMonthYearBackdropInNarrowPane={isInNarrowPaneModal} - textInputContainerStyles={textInputContainerStyles} - />, - ); - } - - if (reportField.type === CONST.REPORT_FIELD_TYPES.LIST && !isReadOnly) { - const enabledOptions = reportField.values.filter((_option: string, index: number) => !reportField.disabledOptions.at(index)); - - // A short list is quicker to scan than to search, so the search input is dropped and its height is not reserved. - const shouldShowSearchInput = enabledOptions.length >= CONST.REPORT_FIELD_LIST_SEARCH_THRESHOLD; - - // A short list is easier to read in a stable alphabetical order than one that moves the current value to the - // top, so only a long list is worth reordering to save scrolling. - const shouldPinSelectedOption = enabledOptions.length >= CONST.STANDARD_LIST_ITEM_LIMIT; - - // FilterPopupButton calls this as a plain function during its own render, so it must not use hooks. Everything - // it needs is read from this component's scope. - const renderOptionsPopup: FilterPopupButtonProps['PopoverComponent'] = ({closeOverlay}) => ( - - { - closeOverlay(); - saveSelectedOption(form[fieldKey] ?? ''); - }} - /> - - ); - - return renderWithDeleteButton( - // No `popoverWidth`, so the dropdown falls back to `CONST.POPOVER_DROPDOWN_WIDTH`, the same width every - // Spend filter dropdown uses. Matching the field's own width instead made narrow fields open odd, cramped - // popovers and wide ones open oversized. - { - const openOptions = (event: GestureResponderEvent | KeyboardEvent) => { - if (event === lastOptionsPressEventRef.current) { - return; - } - lastOptionsPressEventRef.current = event; - onPress(); - }; - - return ( - - - - ); - }} - />, - ); - } - - return renderWithDeleteButton( - textInputRef.current?.blur()} - ref={textInputRef} - textInputContainerStyles={textInputContainerStyles} - />, - ); -} - -export default ReportFieldInlineInput; diff --git a/src/components/ReportActionItem/MoneyReportView.tsx b/src/components/ReportActionItem/MoneyReportView.tsx index 789ce2cba5fd..07d679851ffa 100644 --- a/src/components/ReportActionItem/MoneyReportView.tsx +++ b/src/components/ReportActionItem/MoneyReportView.tsx @@ -1,11 +1,13 @@ import ActivityIndicator from '@components/ActivityIndicator'; import Icon from '@components/Icon'; -import MoneyRequestViewReportFields from '@components/MoneyRequestReportView/MoneyRequestViewReportFields'; +import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription'; +import OfflineWithFeedback from '@components/OfflineWithFeedback'; import SpacerView from '@components/SpacerView'; import Text from '@components/Text'; import UnreadActionIndicator from '@components/UnreadActionIndicator'; import {useCurrencyListActions} from '@hooks/useCurrencyList'; +import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; @@ -15,15 +17,22 @@ import useStyleUtils from '@hooks/useStyleUtils'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import {resolveReportFieldValue} from '@libs/Formula'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {isSingleTransactionReport} from '@libs/MoneyRequestReportUtils'; +import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; +import Navigation from '@libs/Navigation/Navigation'; import {isPolicyTaxEnabled} from '@libs/PolicyUtils'; import { getBillableAndTaxTotal, + getFieldViolation, + getFieldViolationTranslation, getMoneyRequestSpendBreakdown, + getReportFieldKey, getReportFieldMaps, hasUpdatedTotal, isClosedExpenseReportWithNoExpenses as isClosedExpenseReportWithNoExpensesReportUtils, + isReportFieldDisabledForUser, isReportFieldTargetMatchingReport, isSettled as isSettledReportUtils, shouldDisplayReportFields as shouldDisplayReportFieldsUtils, @@ -36,13 +45,16 @@ import AnimatedEmptyStateBackground from '@pages/inbox/report/AnimatedEmptyState import {fontScale} from '@styles/typography'; import type {TranslationPaths} from '@src/languages/types'; +import {clearReportFieldKeyErrors} from '@src/libs/actions/Report'; import ONYXKEYS from '@src/ONYXKEYS'; +import {DYNAMIC_ROUTES} from '@src/ROUTES'; import type {Policy, Report} from '@src/types/onyx'; import type {PendingAction} from '@src/types/onyx/OnyxCommon'; import type {StyleProp, TextStyle} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; +import {Str} from 'expensify-common'; import React, {useMemo} from 'react'; import {View} from 'react-native'; @@ -83,11 +95,13 @@ function MoneyReportView({ }: MoneyReportViewProps) { const theme = useTheme(); const styles = useThemeStyles(); + const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${getNonEmptyStringOnyxID(report?.reportID)}`); const StyleUtils = useStyleUtils(); const {translate} = useLocalize(); - const {convertToDisplayString} = useCurrencyListActions(); + const {convertToDisplayString, getCurrencyDecimals} = useCurrencyListActions(); const {isOffline} = useNetwork(); + const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const isSettled = isSettledReportUtils(report?.reportID); const isTotalUpdated = hasUpdatedTotal(report, policy) && !isTotalPending; @@ -122,12 +136,15 @@ function MoneyReportView({ StyleUtils.getColorStyle(theme.textSupporting), ]; - const policyReportFields = useMemo(() => { - const {fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); - return Object.values(fieldsByName).filter((field) => isReportFieldTargetMatchingReport(report, field)); + const {sortedPolicyReportFields, fieldValues, fieldsByName} = useMemo(() => { + const {fieldValues: values, fieldsByName: byName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); + const sorted = Object.values(byName) + .filter((field) => isReportFieldTargetMatchingReport(report, field)) + .sort(({orderWeight: a}, {orderWeight: b}) => a - b); + return {sortedPolicyReportFields: sorted, fieldValues: values, fieldsByName: byName}; }, [policy?.fieldList, report, reportNameValuePairs]); - const isOnlyTitleFieldEnabled = policyReportFields.every(shouldHideSingleReportField); + const isOnlyTitleFieldEnabled = sortedPolicyReportFields.every(shouldHideSingleReportField); const isClosedExpenseReportWithNoExpenses = isClosedExpenseReportWithNoExpensesReportUtils(report); const shouldDisplayReportFields = shouldDisplayReportFieldsUtils(report, policy); const shouldShowReportField = !isClosedExpenseReportWithNoExpenses && shouldDisplayReportFields && !isOnlyTitleFieldEnabled; @@ -157,17 +174,51 @@ function MoneyReportView({ {shouldShowAnimatedBackground && } {!isClosedExpenseReportWithNoExpenses && ( <> - {/* One-expense reports edit their fields in place like the multi-expense report view does, one field - per row since this screen reads as a single expense rather than a table. */} - {shouldDisplayReportFields && (!isCombinedReport || !isOnlyTitleFieldEnabled) && ( - - )} + {shouldDisplayReportFields && + (!isCombinedReport || !isOnlyTitleFieldEnabled) && + sortedPolicyReportFields.map((reportField) => { + if (shouldHideSingleReportField(reportField)) { + return null; + } + + const fieldValue = resolveReportFieldValue(reportField, report, policy, fieldValues, fieldsByName, getCurrencyDecimals); + const isFieldDisabled = isReportFieldDisabledForUser(report, reportField, policy, currentUserAccountID, rules); + const fieldKey = getReportFieldKey(reportField.fieldID); + + const violation = isFieldDisabled ? undefined : getFieldViolation(reportField); + const violationTranslation = getFieldViolationTranslation(reportField, violation); + + return ( + clearReportFieldKeyErrors(report?.reportID, fieldKey)} + > + { + if (!report?.policyID) { + return; + } + + Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.EDIT_REPORT_FIELD.getRoute(report.policyID, reportField.fieldID))); + }} + shouldShowRightIcon={!isFieldDisabled} + wrapperStyle={[styles.pv2, styles.taskDescriptionMenuItem]} + shouldGreyOutWhenDisabled={false} + numberOfLinesTitle={0} + interactive={!isFieldDisabled} + titleWithTooltips={[]} + brickRoadIndicator={violation ? 'error' : undefined} + errorText={violationTranslation} + /> + + ); + })} {shouldShowTotalRow && ( diff --git a/src/components/Search/FilterDropdowns/FilterPopupButton.tsx b/src/components/Search/FilterDropdowns/FilterPopupButton.tsx index 351f81a43ee1..5bf8eff98616 100644 --- a/src/components/Search/FilterDropdowns/FilterPopupButton.tsx +++ b/src/components/Search/FilterDropdowns/FilterPopupButton.tsx @@ -67,11 +67,7 @@ function FilterPopupButton({ const isFocused = useIsFocused(); const styles = useThemeStyles(); const {isKeyboardActive} = useKeyboardState(); - // The style is applied inside the popover, which never renders an offline indicator, so don't reserve space for one - const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({ - addBottomSafeAreaPadding: isSmallScreenWidth && !isKeyboardActive, - addOfflineIndicatorBottomSafeAreaPadding: false, - }); + const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: isSmallScreenWidth && !isKeyboardActive}); const StyleUtils = useStyleUtils(); const {windowHeight} = useWindowDimensions(); const triggerRef = useRef | null>(null); diff --git a/src/hooks/useSaveReportField.ts b/src/hooks/useSaveReportField.ts deleted file mode 100644 index cba5052bed1a..000000000000 --- a/src/hooks/useSaveReportField.ts +++ /dev/null @@ -1,81 +0,0 @@ -/** - * Builds the callback that persists a report field value. - * - * Both the report field editor page and the inline report field inputs in the report view save through this, so the - * violation, beta and recently-used handling only has to exist in one place. - */ - -import {useSession} from '@components/OnyxListItemProvider'; - -import {updateReportField, updateReportName} from '@libs/actions/Report'; -import {getReportFieldKey, hasViolations as hasViolationsReportUtils, isReportFieldDisabled, isReportFieldOfTypeTitle} from '@libs/ReportUtils'; - -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; -import type {Policy, PolicyReportField, Report} from '@src/types/onyx'; - -import type {OnyxEntry} from 'react-native-onyx'; - -import {isTrackIntentUserSelector} from '@selectors/Onboarding'; - -import useOnyx from './useOnyx'; -import usePermissions from './usePermissions'; -import useReportTransactions from './useReportTransactions'; -import useReportTransactionViolations from './useReportTransactionViolations'; - -function useSaveReportField(report: OnyxEntry, policy: OnyxEntry) { - const session = useSession(); - const {isBetaEnabled} = usePermissions(); - const [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); - // Both of these are scoped to this report so a transaction or violation change in an unrelated report does not re-render every consumer. - const reportTransactions = useReportTransactions(report?.reportID); - const [transactionViolations] = useReportTransactionViolations(reportTransactions); - const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); - const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); - - return (reportField: PolicyReportField, value: string) => { - if (!report) { - return; - } - - // Only a text field can be cleared, the same as on OldDot. A report always has a name, so the title can never be - // empty, and the other field types have no way to enter an empty value. - if (value === '' && (isReportFieldOfTypeTitle(reportField) || reportField.type !== CONST.REPORT_FIELD_TYPES.TEXT)) { - return; - } - - if (isReportFieldOfTypeTitle(reportField)) { - updateReportName(report.reportID, value, report.reportName ?? ''); - return; - } - - const fieldKey = getReportFieldKey(reportField.fieldID); - const hasViolations = hasViolationsReportUtils( - report.reportID, - transactionViolations, - session?.accountID ?? CONST.DEFAULT_NUMBER_ID, - session?.email ?? '', - undefined, - reportTransactions, - ); - const isFieldDisabled = isReportFieldDisabled(report, reportField, policy, rules); - const hasOtherViolations = Object.entries(report.fieldList ?? {}).some(([key, field]) => key !== fieldKey && field.value === '' && !isFieldDisabled); - - updateReportField({ - report: {...report, reportID: report.reportID}, - reportField: {...reportField, value}, - previousReportField: reportField, - policy, - isASAPSubmitBetaEnabled: isBetaEnabled(CONST.BETAS.ASAP_SUBMIT), - accountID: session?.accountID ?? CONST.DEFAULT_NUMBER_ID, - email: session?.email ?? '', - hasViolationsParam: hasViolations, - recentlyUsedReportFields, - shouldFixViolations: hasOtherViolations, - isTrackIntentUser, - rules, - }); - }; -} - -export default useSaveReportField; diff --git a/src/languages/de.ts b/src/languages/de.ts index 82bd9b0e1791..6538abb6f7c6 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -10540,7 +10540,6 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `${fieldName} auf „${newValue}“ geändert (zuvor „${oldValue}“)`, changeFieldEmpty: (newValue: string, fieldName: string) => `setze ${fieldName} auf „${newValue}“`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `${fieldName} entfernt (zuvor „${oldValue}“)`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `hat den Arbeitsbereich${fromPolicyName ? `(zuvor ${fromPolicyName})` : ''} geändert`; diff --git a/src/languages/el.ts b/src/languages/el.ts index 7f81db025b46..0ae55c3072aa 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -10740,7 +10740,6 @@ ${reportName}`, type: { changeField: (oldValue, newValue, fieldName) => `άλλαξε το ${fieldName} σε «${newValue}» (προηγουμένως «${oldValue}»)`, changeFieldEmpty: (newValue, fieldName) => `ορίστε το ${fieldName} σε «${newValue}»`, - changeFieldRemoved: (oldValue, fieldName) => `αφαίρεσε το ${fieldName} (προηγουμένως «${oldValue}»)`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `άλλαξε τον χώρο εργασίας${fromPolicyName ? `(προηγουμένως ${fromPolicyName})` : ''}`; diff --git a/src/languages/en.ts b/src/languages/en.ts index 2b7fcd1c2931..650b9b70bedc 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -10775,7 +10775,6 @@ const translations = { type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `changed ${fieldName} to "${newValue}" (previously "${oldValue}")`, changeFieldEmpty: (newValue: string, fieldName: string) => `set ${fieldName} to "${newValue}"`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `removed ${fieldName} (previously "${oldValue}")`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `changed the workspace${fromPolicyName ? ` (previously ${fromPolicyName})` : ''}`; diff --git a/src/languages/es.ts b/src/languages/es.ts index b08662eba412..d9a4099f6cf2 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -10342,7 +10342,6 @@ ${reportName}`, type: { changeField: (oldValue, newValue, fieldName) => `cambió ${fieldName} a "${newValue}" (previamente "${oldValue}")`, changeFieldEmpty: (newValue, fieldName) => `estableció ${fieldName} a ${newValue}`, - changeFieldRemoved: (oldValue, fieldName) => `eliminó ${fieldName} (previamente "${oldValue}")`, changeReportPolicy: (toPolicyName, fromPolicyName) => { if (!toPolicyName) { return `cambió el espacio de trabajo${fromPolicyName ? ` (previamente ${fromPolicyName})` : ''}`; diff --git a/src/languages/fr.ts b/src/languages/fr.ts index 4fd64657187f..031ffb57f5d3 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -10566,7 +10566,6 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `a modifié ${fieldName} en « ${newValue} » (auparavant « ${oldValue} »)`, changeFieldEmpty: (newValue: string, fieldName: string) => `définir ${fieldName} sur « ${newValue} »`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `a supprimé ${fieldName} (auparavant « ${oldValue} »)`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `a modifié l’espace de travail${fromPolicyName ? `(précédemment ${fromPolicyName})` : ''}`; diff --git a/src/languages/it.ts b/src/languages/it.ts index 7c1432ea8acb..eb6db0f38534 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -10488,7 +10488,6 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `ha modificato ${fieldName} in "${newValue}" (in precedenza "${oldValue}")`, changeFieldEmpty: (newValue: string, fieldName: string) => `imposta ${fieldName} su "${newValue}"`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `ha rimosso ${fieldName} (in precedenza "${oldValue}")`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `ha modificato il workspace${fromPolicyName ? `(precedentemente ${fromPolicyName})` : ''}`; diff --git a/src/languages/ja.ts b/src/languages/ja.ts index 8f9a09d1ce05..ae1b7537de61 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -10334,7 +10334,6 @@ ${reportName}`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `${fieldName} を「${newValue}」（以前は「${oldValue}」）に変更しました`, changeFieldEmpty: (newValue: string, fieldName: string) => `${fieldName} を「${newValue}」に設定`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `${fieldName} を削除しました（以前は「${oldValue}」）`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `ワークスペース${fromPolicyName ? `（以前は${fromPolicyName}）` : ''}を変更しました`; diff --git a/src/languages/nl.ts b/src/languages/nl.ts index 0ee73130d9ed..bb53f310db18 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -10458,7 +10458,6 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `heeft ${fieldName} gewijzigd in „${newValue}” (voorheen „${oldValue}”)`, changeFieldEmpty: (newValue: string, fieldName: string) => `stel ${fieldName} in op "${newValue}"`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `heeft ${fieldName} verwijderd (voorheen „${oldValue}”)`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `heeft de werkruimte${fromPolicyName ? `(voorheen ${fromPolicyName})` : ''} gewijzigd`; diff --git a/src/languages/pl.ts b/src/languages/pl.ts index 32e71b269905..757b4c2c76ef 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -10473,7 +10473,6 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `zmieniono ${fieldName} na „${newValue}” (wcześniej „${oldValue}”)`, changeFieldEmpty: (newValue: string, fieldName: string) => `ustaw ${fieldName} na „${newValue}”`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `usunięto ${fieldName} (wcześniej „${oldValue}”)`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `zmieniono przestrzeń roboczą${fromPolicyName ? `(wcześniej ${fromPolicyName})` : ''}`; diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index 1be8926ac6de..45ce91d1a319 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -10455,7 +10455,6 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `alterou ${fieldName} para "${newValue}" (antes "${oldValue}")`, changeFieldEmpty: (newValue: string, fieldName: string) => `definir ${fieldName} como "${newValue}"`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `removeu ${fieldName} (antes "${oldValue}")`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `alterou o espaço de trabalho${fromPolicyName ? `(antes ${fromPolicyName})` : ''}`; diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index e684d9867afc..9075df6e68f4 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -10054,7 +10054,6 @@ ${reportName}`, type: { changeField: (oldValue: string | undefined, newValue: string, fieldName: string) => `将${fieldName}更改为“${newValue}”（先前为“${oldValue}”）`, changeFieldEmpty: (newValue: string, fieldName: string) => `将 ${fieldName} 设置为“${newValue}”`, - changeFieldRemoved: (oldValue: string, fieldName: string) => `已移除${fieldName}（先前为“${oldValue}”）`, changeReportPolicy: (toPolicyName: string, fromPolicyName?: string) => { if (!toPolicyName) { return `更改了工作区${fromPolicyName ? `（原为 ${fromPolicyName}）` : ''}`; diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index eceeea54af6f..e4ef4322377d 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -2432,9 +2432,6 @@ function getMessageOfOldDotReportAction(translate: LocalizedTranslate, oldDotAct switch (actionName) { case CONST.REPORT.ACTIONS.TYPE.CHANGE_FIELD: { const {oldValue, newValue, fieldName} = originalMessage; - if (oldValue && !newValue) { - return translate('report.actions.type.changeFieldRemoved', oldValue, fieldName); - } if (!oldValue) { return translate('report.actions.type.changeFieldEmpty', newValue, fieldName); } diff --git a/src/libs/ReportFieldOptionsListUtils.ts b/src/libs/ReportFieldOptionsListUtils.ts index 912280000169..610c1bdfe700 100644 --- a/src/libs/ReportFieldOptionsListUtils.ts +++ b/src/libs/ReportFieldOptionsListUtils.ts @@ -30,8 +30,6 @@ function getReportFieldOptionsSection({ initiallySelectedValue, searchValue, translate, - shouldPinSelectedOption = true, - shouldShowSectionTitles = true, }: { options: string[]; recentlyUsedOptions: string[]; @@ -39,18 +37,12 @@ function getReportFieldOptionsSection({ initiallySelectedValue: string; searchValue: string; translate: LocalizedTranslate; - - /** Whether the selected value is lifted into its own section at the top of the list */ - shouldPinSelectedOption?: boolean; - - /** Whether the "Recent" and "All" section titles are rendered */ - shouldShowSectionTitles?: boolean; }) { const reportFieldOptionsSections = []; // The live selection drives the checkmark (isSelected)... const selectedKeySet = new Set(selectedOptions.map(({text, keyForList, name}) => text ?? keyForList ?? name ?? '').filter((o) => !!o)); // ...while the frozen initial selection drives the pinned "Selected" section, so selecting a value marks it without reordering the list until reopen. - const pinnedOptionKeys = shouldPinSelectedOption && initiallySelectedValue ? [initiallySelectedValue] : []; + const pinnedOptionKeys = initiallySelectedValue ? [initiallySelectedValue] : []; const pinnedKeySet = new Set(pinnedOptionKeys); if (searchValue) { @@ -89,7 +81,7 @@ function getReportFieldOptionsSection({ if (filteredRecentlyUsedOptions.length > 0) { reportFieldOptionsSections.push({ // "Recent" section - title: shouldShowSectionTitles ? translate('common.recent') : '', + title: translate('common.recent'), sectionIndex: 2, data: getReportFieldOptions(filteredRecentlyUsedOptions, selectedKeySet), }); @@ -97,7 +89,7 @@ function getReportFieldOptionsSection({ reportFieldOptionsSections.push({ // "All" section when items amount more than the threshold - title: shouldShowSectionTitles ? translate('common.all') : '', + title: translate('common.all'), sectionIndex: 3, data: getReportFieldOptions(filteredOptions, selectedKeySet), }); diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 4f771a2dbbbe..ab89b15200c3 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -4152,7 +4152,7 @@ function updateReportField({ report: Report; reportField: PolicyReportField; previousReportField: PolicyReportField; - policy: OnyxEntry; + policy: Policy; isASAPSubmitBetaEnabled: boolean; accountID: number; email: string; diff --git a/src/pages/DynamicEditReportFieldPage.tsx b/src/pages/DynamicEditReportFieldPage.tsx index f24e212352c5..6a63a50e7a10 100644 --- a/src/pages/DynamicEditReportFieldPage.tsx +++ b/src/pages/DynamicEditReportFieldPage.tsx @@ -2,6 +2,7 @@ import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView import type {FormOnyxValues} from '@components/Form/types'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import {ModalActions} from '@components/Modal/Global/ModalContext'; +import {useSession} from '@components/OnyxListItemProvider'; import type {PopoverMenuItem} from '@components/PopoverMenu'; import ScreenWrapper from '@components/ScreenWrapper'; @@ -11,10 +12,10 @@ import useDynamicBackPath from '@hooks/useDynamicBackPath'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePermissions from '@hooks/usePermissions'; import {useDerivedReportNameByReportID} from '@hooks/useReportAttributes'; -import useSaveReportField from '@hooks/useSaveReportField'; -import {deleteReportField} from '@libs/actions/Report'; +import {deleteReportField, updateReportField, updateReportName} from '@libs/actions/Report'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; import type {EditRequestNavigatorParamList} from '@libs/Navigation/types'; @@ -24,8 +25,10 @@ import { getReportFieldFromReportNameValuePairs, getReportFieldKey, getTitleFieldWithFallback, + hasViolations as hasViolationsReportUtils, isGroupPolicyExpenseReport, isInvoiceReport, + isReportFieldDisabled, isReportFieldDisabledForUser, isReportFieldOfTypeTitle, isReportFieldTargetMatchingReport, @@ -35,7 +38,9 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import {DYNAMIC_ROUTES} from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; +import type {Policy} from '@src/types/onyx'; +import {isTrackIntentUserSelector} from '@selectors/Onboarding'; import {Str} from 'expensify-common'; import React from 'react'; @@ -53,7 +58,7 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${reportID}`); const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); const derivedReportName = useDerivedReportNameByReportID(reportID); - const saveReportField = useSaveReportField(report, policy); + const [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); const isTitleField = route.params.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID; const reportFieldFromNVP = getReportFieldFromReportNameValuePairs(reportNameValuePairs, fieldKey) ?? getReportFieldFromReportNameValuePairs(reportNameValuePairs, route.params.fieldID); @@ -71,12 +76,20 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails(); const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const isDisabled = isReportFieldDisabledForUser(report, reportField, policy, currentUserAccountID, rules) && reportField?.type !== CONST.REPORT_FIELD_TYPES.FORMULA; + const {isBetaEnabled} = usePermissions(); + const isASAPSubmitBetaEnabled = isBetaEnabled(CONST.BETAS.ASAP_SUBMIT); + const session = useSession(); + const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS); + const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector}); + const hasViolations = hasViolationsReportUtils(report?.reportID, transactionViolations, session?.accountID ?? CONST.DEFAULT_NUMBER_ID, session?.email ?? ''); const {translate} = useLocalize(); const {showConfirmModal} = useConfirmModal(); const icons = useMemoizedLazyExpensifyIcons(['Trashcan']); const isReportFieldTitle = isReportFieldOfTypeTitle(reportField); const isReportFieldsFeatureEnabled = report?.type === CONST.REPORT.TYPE.INVOICE ? policy?.areInvoiceFieldsEnabled : policy?.areReportFieldsEnabled; const reportFieldsEnabled = ((isGroupPolicyExpenseReport(report, policy?.type) || isInvoiceReport(report)) && !!isReportFieldsFeatureEnabled) || isReportFieldTitle; + const hasOtherViolations = + report?.fieldList && Object.entries(report.fieldList).some(([key, field]) => key !== fieldKey && field.value === '' && !isReportFieldDisabled(report, reportField, policy, rules)); if (!reportFieldsEnabled || !reportField || !policyField || !report || !isReportFieldTargetMatchingReport(report, reportField) || isDisabled) { return ( @@ -127,8 +140,28 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { return; } - saveReportField(reportField, value); - goBack(); + if (isReportFieldTitle) { + updateReportName(report.reportID, value, report.reportName ?? ''); + goBack(); + } else { + if (value !== '') { + updateReportField({ + report: {...report, reportID: report.reportID}, + reportField: {...reportField, value}, + previousReportField: reportField, + policy: policy as unknown as Policy, + isASAPSubmitBetaEnabled, + accountID: session?.accountID ?? CONST.DEFAULT_NUMBER_ID, + email: session?.email ?? '', + hasViolationsParam: hasViolations, + recentlyUsedReportFields, + shouldFixViolations: hasOtherViolations ?? false, + isTrackIntentUser, + rules, + }); + } + goBack(); + } }; const menuItems: PopoverMenuItem[] = []; diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index 166332ce0d83..038d01ca64de 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -5,7 +5,6 @@ import useDebouncedState from '@hooks/useDebouncedState'; import useInitialSelection from '@hooks/useInitialSelection'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; -import useThemeStyles from '@hooks/useThemeStyles'; import {getHeaderMessageForNonUserList} from '@libs/OptionsListUtils'; import {getReportFieldOptionsSection} from '@libs/ReportFieldOptionsListUtils'; @@ -26,39 +25,13 @@ type EditReportFieldDropdownPageProps = { /** Callback to fire when the Save button is pressed */ onSubmit: (form: Record) => void; - - /** Whether the search input is shown. Callers with a short list hide it, since there is little to search through. */ - shouldShowTextInput?: boolean; - - /** Whether the "Recent" section is shown above the full option list */ - shouldShowRecentlyUsedOptions?: boolean; - - /** Whether the selected value is lifted to the top of the list. Callers with a short list leave it in place, so the order doesn't change as the value does. */ - shouldPinSelectedOption?: boolean; - - /** Whether the "Recent" and "All" section titles are rendered */ - shouldShowSectionTitles?: boolean; - - /** Whether options use the shorter 52px row the Spend dropdowns use, instead of the default 64px page row */ - shouldUseCompactRows?: boolean; }; -function EditReportFieldDropdown({ - onSubmit, - fieldKey, - fieldValue, - fieldOptions, - shouldShowTextInput = true, - shouldShowRecentlyUsedOptions = true, - shouldPinSelectedOption = true, - shouldShowSectionTitles = true, - shouldUseCompactRows = false, -}: EditReportFieldDropdownPageProps) { - const styles = useThemeStyles(); +function EditReportFieldDropdown({onSubmit, fieldKey, fieldValue, fieldOptions}: EditReportFieldDropdownPageProps) { const [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState(''); const {translate, localeCompare} = useLocalize(); - const recentlyUsedOptions = shouldShowRecentlyUsedOptions ? (recentlyUsedReportFields?.[fieldKey]?.sort(localeCompare) ?? []) : []; + const recentlyUsedOptions = recentlyUsedReportFields?.[fieldKey]?.sort(localeCompare) ?? []; const validFieldOptions = fieldOptions?.filter((option) => !!option)?.sort(localeCompare); @@ -66,7 +39,7 @@ function EditReportFieldDropdown({ // The live value still drives the checkmark, so tapping a row marks it without reordering the list. The reorder happens only on reopen. const initialFieldValue = useInitialSelection(fieldValue, {resetOnFocus: true}); - const optionSections = getReportFieldOptionsSection({ + const sections = getReportFieldOptionsSection({ searchValue: debouncedSearchValue, // Live value drives the checkmark, so tapping a row marks it immediately. selectedOptions: [ @@ -81,16 +54,8 @@ function EditReportFieldDropdown({ options: validFieldOptions, recentlyUsedOptions, translate, - shouldPinSelectedOption, - shouldShowSectionTitles, }); - // Same override the Spend single-select dropdowns use to shorten the default 64px option row. It also makes the rows - // match the 52px `getSelectionListPopoverHeight` already assumes, so the popover stops being sized for less content - // than it holds. Without vertical padding a two line title still fits the row height. - const compactRowStyle = [styles.optionRowCompactMinHeight, styles.pv0]; - const sections = shouldUseCompactRows ? optionSections.map((section) => ({...section, data: section.data.map((option) => ({...option, itemStyle: compactRowStyle}))})) : optionSections; - const policyReportFieldData = sections.at(0)?.data ?? []; const textInputOptions = { @@ -98,21 +63,15 @@ function EditReportFieldDropdown({ label: translate('common.search'), onChangeText: setSearchValue, headerMessage: getHeaderMessageForNonUserList(translate, policyReportFieldData.length > 0, debouncedSearchValue), - // Nothing is rendered above the search input here, so it needs the top padding the surrounding page or popover doesn't provide. - style: {containerStyle: styles.pt3}, }; return ( onSubmit({[fieldKey]: option?.text ?? ''})} + onSelectRow={(option) => onSubmit({[fieldKey]: !option?.text || fieldValue === option.text ? '' : option.text})} initiallyFocusedItemKey={initialFieldValue} shouldUpdateFocusedIndex /> diff --git a/src/styles/index.ts b/src/styles/index.ts index 66ed1d62cfd4..5c654339f454 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -2277,26 +2277,6 @@ const staticStyles = (theme: ThemeColors) => paddingBottom: 12, }, - optionRowCompactMinHeight: { - minHeight: variables.optionRowHeightCompact, - }, - - // Sits flush against the right edge of a report field whose input has `noRightBorderRadius`, so the input's - // right border reads as a divider and the two look like one segmented field. - reportFieldDeleteButton: { - width: variables.componentSizeLarge, - // Matches the input beside it, whose height follows the device font scale - height: variables.inputHeight, - alignItems: 'center', - justifyContent: 'center', - backgroundColor: theme.appBG, - borderWidth: 1, - borderLeftWidth: 0, - borderTopRightRadius: variables.componentBorderRadiusNormal, - borderBottomRightRadius: variables.componentBorderRadiusNormal, - borderColor: theme.bordersBold, - }, - optionsListSectionHeader: { marginTop: 8, marginBottom: 4, @@ -3425,14 +3405,6 @@ const staticStyles = (theme: ThemeColors) => transform: [{rotate: '180deg'}], }, - // Use this instead of `flipUpsideDown` when the element being flipped has asymmetric horizontal padding. - // `rotate(180deg)` turns the whole box about its centre, so it mirrors that padding too and the content - // visibly slides sideways. Mirroring on the vertical axis leaves the box where it is, and looks identical - // for content that is already symmetric left-to-right (a caret, a chevron, a tooltip pointer). - flipUpsideDownInPlace: { - transform: [{scaleY: -1}], - }, - navigationScreenCardStyle: { height: '100%', }, diff --git a/tests/ui/EditReportFieldDropdownTest.tsx b/tests/ui/EditReportFieldDropdownTest.tsx deleted file mode 100644 index 9bc104cc7955..000000000000 --- a/tests/ui/EditReportFieldDropdownTest.tsx +++ /dev/null @@ -1,131 +0,0 @@ -import {act, fireEvent, render, screen} from '@testing-library/react-native'; - -import ComposeProviders from '@components/ComposeProviders'; -import OnyxListItemProvider from '@components/OnyxListItemProvider'; - -import EditReportFieldDropdown from '@pages/EditReportFieldDropdown'; - -import ONYXKEYS from '@src/ONYXKEYS'; - -import type * as NativeNavigation from '@react-navigation/native'; - -import React from 'react'; -import Onyx from 'react-native-onyx'; - -import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; - -jest.mock('@react-navigation/native', () => ({ - ...((): typeof NativeNavigation => jest.requireActual('@react-navigation/native'))(), - useNavigation: jest.fn(() => ({navigate: jest.fn(), addListener: jest.fn(() => jest.fn())})), - useIsFocused: jest.fn(() => true), - useRoute: jest.fn(() => ({key: '', name: ''})), - // The dropdown is rendered outside a navigator here, so the focus effect that resets the pinned selection is a noop. - useFocusEffect: jest.fn(), -})); - -const fieldKey = 'expensify_field_dropdown'; -const fieldOptions = ['Alpha', 'Bravo', 'Charlie']; - -const renderDropdown = async (props: Partial> = {}) => { - const rendered = render( - - - , - ); - await waitForBatchedUpdatesWithAct(); - return rendered; -}; - -describe('EditReportFieldDropdown', () => { - beforeAll(() => { - Onyx.init({keys: ONYXKEYS}); - }); - - beforeEach(async () => { - await act(async () => { - // "Bravo" was used recently, so the "Recent" section has something to render when it is enabled. - await Onyx.merge(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS, {[fieldKey]: ['Bravo']}); - }); - await waitForBatchedUpdatesWithAct(); - }); - - afterEach(async () => { - await act(async () => { - await Onyx.clear(); - }); - }); - - it('shows the search input by default', async () => { - // Given a dropdown rendered without any opt-out - await renderDropdown(); - - // When the list is displayed - // Then the search input is there, because the editor page has always been searchable and the new flags must - // not quietly change what an existing caller gets - expect(screen.getByTestId('selection-list-text-input')).toBeOnTheScreen(); - }); - - it('hides the search input when the caller opts out', async () => { - // Given a caller that opts out of the search input, which the inline field does for a short option list - await renderDropdown({shouldShowTextInput: false}); - - // When the list is displayed - // Then the search input is gone, so the popover does not reserve height for a box that is quicker to scan - // past than to type into - expect(screen.queryByTestId('selection-list-text-input')).not.toBeOnTheScreen(); - }); - - it('lists a recently used option twice by default, once in the "Recent" section and once in the full list', async () => { - // Given "Bravo" recorded as recently used and a dropdown rendered without any opt-out - await renderDropdown(); - - // When the list is displayed - // Then "Bravo" appears in both the "Recent" section and the full list, which is the editor page's existing - // behavior and must survive the flags being added - expect(screen.getAllByTestId('base-list-item-Bravo')).toHaveLength(2); - }); - - it('lists a recently used option only once when the caller opts out of the "Recent" section', async () => { - // Given "Bravo" recorded as recently used and a caller that opts out of the "Recent" section - await renderDropdown({shouldShowRecentlyUsedOptions: false}); - - // When the list is displayed - // Then "Bravo" appears once. A short list shown in a small popover is worse for having the same few options - // repeated at the top than it is for making the user read all of them - expect(screen.getAllByTestId('base-list-item-Bravo')).toHaveLength(1); - }); - - it('submits the option that was picked', async () => { - // Given a dropdown whose field currently holds "Alpha" - const onSubmit = jest.fn(); - await renderDropdown({onSubmit}); - - // When a different option is pressed - fireEvent.press(screen.getByTestId('base-list-item-Charlie')); - await waitForBatchedUpdatesWithAct(); - - // Then it is submitted under the field's own key, which is what both the editor page and the inline field - // rely on to know which field the value belongs to - expect(onSubmit).toHaveBeenCalledWith({[fieldKey]: 'Charlie'}); - }); - - it('submits the current value rather than an empty string when the option already selected is picked again', async () => { - // Given a dropdown whose field currently holds "Alpha" - const onSubmit = jest.fn(); - await renderDropdown({onSubmit}); - - // When that same option is pressed again - fireEvent.press(screen.getByTestId('base-list-item-Alpha')); - await waitForBatchedUpdatesWithAct(); - - // Then the current value is submitted, not an empty string. Callers skip a no-op save by comparing this - // against the stored value, so an empty string would be read as the user having cleared the field - expect(onSubmit).toHaveBeenCalledWith({[fieldKey]: 'Alpha'}); - }); -}); diff --git a/tests/ui/MoneyReportViewTest.tsx b/tests/ui/MoneyReportViewTest.tsx index eb040b9a2724..5d525c985783 100644 --- a/tests/ui/MoneyReportViewTest.tsx +++ b/tests/ui/MoneyReportViewTest.tsx @@ -19,20 +19,6 @@ import * as LHNTestUtils from '../utils/LHNTestUtils'; import * as TestHelper from '../utils/TestHelper'; import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; -// Pinned to a wide layout so the column-count assertion below exercises the `shouldUseSingleColumn` prop rather than -// the narrow-layout fallback, which would collapse the fields to one per row on its own. -jest.mock('@hooks/useResponsiveLayoutOnWideRHP', () => ({ - __esModule: true, - default: () => ({ - shouldUseNarrowLayout: false, - shouldUseNarrowLayoutIgnoringWideRHP: false, - isSmallScreenWidth: false, - isInNarrowPaneModal: false, - isWideRHPDisplayedOnWideLayout: true, - isSuperWideRHPDisplayedOnWideLayout: false, - }), -})); - jest.mock('@hooks/useLocalize', () => jest.fn(() => ({ translate: jest.fn((key: string) => key), @@ -355,38 +341,6 @@ describe('MoneyReportView report fields visibility', () => { }); }); - const buildFourFieldReport = () => { - const customFields = Array.from({length: 4}, (_unused, index) => ({ - ...buildCustomTextField(), - fieldID: `field_test${index}`, - name: `Test${index}`, - orderWeight: index + 2, - })); - const fieldList: Record = {[CONST.REPORT_FIELD_TITLE_FIELD_ID]: buildTitleField()}; - for (const field of customFields) { - fieldList[`expensify_${field.fieldID}`] = field; - } - return {policy: buildReportFieldsPolicy(fieldList), report: buildExpenseReport({fieldList})}; - }; - - it('edits the report fields inline, one per row on a wide layout', async () => { - // Given a one-expense report with 4 custom fields on a wide layout - const {policy, report} = buildFourFieldReport(); - await seedReportFieldsPolicy(policy, report); - - // When this view renders them - renderMoneyReportView(report, policy); - await waitForBatchedUpdatesWithAct(); - - // Then each field is an input on its own row. Four fields would fill two rows of three if the grid were in use, - // so four rows proves this view asks for a single column: it renders inside a container too narrow for three - // fields side by side even though the window itself is wide - await waitFor(() => { - expect(screen.getAllByTestId('reportFieldsRow')).toHaveLength(4); - }); - expect(screen.getByLabelText('Test0')).toBeOnTheScreen(); - }); - it('keeps a custom report field visible for a non-admin submitter after the report is approved (single-expense combined view)', async () => { const fieldList = { [CONST.REPORT_FIELD_TITLE_FIELD_ID]: buildTitleField(), @@ -405,7 +359,7 @@ describe('MoneyReportView report fields visibility', () => { // The custom field (rendered read-only after approval) must still show for the submitter. await waitFor(() => { - expect(screen.getByLabelText('Test')).toBeOnTheScreen(); + expect(screen.getByText('Test')).toBeOnTheScreen(); }); }); diff --git a/tests/ui/MoneyRequestViewReportFieldsTest.tsx b/tests/ui/MoneyRequestViewReportFieldsTest.tsx deleted file mode 100644 index 27d512563a15..000000000000 --- a/tests/ui/MoneyRequestViewReportFieldsTest.tsx +++ /dev/null @@ -1,601 +0,0 @@ -import {act, fireEvent, render, screen} from '@testing-library/react-native'; - -import ComposeProviders from '@components/ComposeProviders'; -import MoneyRequestViewReportFields from '@components/MoneyRequestReportView/MoneyRequestViewReportFields'; -import OnyxListItemProvider from '@components/OnyxListItemProvider'; -import PopoverWithMeasuredContent from '@components/PopoverWithMeasuredContent'; - -import {deleteReportField, updateReportField} from '@libs/actions/Report'; - -import initOnyxDerivedValues from '@userActions/OnyxDerived'; - -import CONST from '@src/CONST'; -import IntlStore from '@src/languages/IntlStore'; -import ONYXKEYS from '@src/ONYXKEYS'; -import type * as OnyxTypes from '@src/types/onyx'; - -import type * as NativeNavigation from '@react-navigation/native'; - -import React from 'react'; -import {StyleSheet} from 'react-native'; -import Onyx from 'react-native-onyx'; - -import * as LHNTestUtils from '../utils/LHNTestUtils'; -import * as TestHelper from '../utils/TestHelper'; -import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; - -let mockShouldUseNarrowLayout = false; - -jest.mock('@hooks/useResponsiveLayoutOnWideRHP', () => ({ - __esModule: true, - default: () => ({ - shouldUseNarrowLayout: mockShouldUseNarrowLayout, - shouldUseNarrowLayoutIgnoringWideRHP: mockShouldUseNarrowLayout, - isSmallScreenWidth: mockShouldUseNarrowLayout, - isInNarrowPaneModal: false, - isWideRHPDisplayedOnWideLayout: !mockShouldUseNarrowLayout, - isSuperWideRHPDisplayedOnWideLayout: false, - }), -})); - -jest.mock('@hooks/useLocalize', () => - jest.fn(() => ({ - translate: jest.fn((key: string) => key), - numberFormat: jest.fn((num: number) => num.toString()), - toLocaleDigit: jest.fn((digit: string) => digit), - localeCompare: jest.fn((first: string, second: string) => first.localeCompare(second)), - })), -); - -jest.mock('@react-navigation/native', () => ({ - ...((): typeof NativeNavigation => jest.requireActual('@react-navigation/native'))(), - useNavigation: jest.fn(() => ({navigate: jest.fn(), addListener: jest.fn(() => jest.fn())})), - useIsFocused: jest.fn(() => true), - useRoute: jest.fn(() => ({key: '', name: '', params: {reportID: '1'}})), - // The option list is rendered outside a navigator here, so there is no focus cycle to refresh its snapshot on. - useFocusEffect: jest.fn(), -})); - -// The list field's dropdown is built on FilterPopupButton, which reads focus straight from @react-navigation/core. -jest.mock('@react-navigation/core', () => ({ - ...jest.requireActual>('@react-navigation/core'), - useIsFocused: jest.fn(() => true), -})); - -jest.mock('@libs/actions/Report', () => ({ - ...jest.requireActual>('@libs/actions/Report'), - updateReportField: jest.fn(), - deleteReportField: jest.fn(), -})); - -const mockShowConfirmModal = jest.fn(() => Promise.resolve({action: 'CONFIRM'})); - -jest.mock('@hooks/useConfirmModal', () => ({ - __esModule: true, - default: () => ({showConfirmModal: mockShowConfirmModal}), -})); - -TestHelper.setupGlobalFetchMock(); - -const policyID = 'policy_report_fields_grid'; -const reportID = 'report_report_fields_grid'; -const accountID = 1; - -const buildTextField = (index: number): OnyxTypes.PolicyReportField => ({ - name: `Field${index}`, - fieldID: `field${index}`, - defaultValue: '', - value: `Value${index}`, - orderWeight: index, - type: CONST.REPORT_FIELD_TYPES.TEXT, - deletable: true, - values: [], - keys: [], - externalIDs: [], - disabledOptions: [], - target: CONST.REPORT_FIELD_TARGETS.EXPENSE, - isTax: false, -}); - -// An empty value is what `getFieldViolation` turns into a " is required" violation. -const buildEmptyField = (): OnyxTypes.PolicyReportField => ({ - ...buildTextField(1), - name: 'EmptyField', - fieldID: 'emptyField', - value: '', -}); - -const buildListField = (): OnyxTypes.PolicyReportField => ({ - ...buildTextField(1), - name: 'ListField', - fieldID: 'listField', - type: CONST.REPORT_FIELD_TYPES.LIST, - value: 'Option1', - values: ['Option1', 'Option2'], - disabledOptions: [false, false], -}); - -// A field the admin deleted from the workspace. The report keeps its own copy, so the value it already held is not lost. -const buildDeletedField = (): OnyxTypes.PolicyReportField => ({ - ...buildTextField(1), - name: 'DeletedField', - fieldID: 'deletedField', - value: 'Old value', -}); - -/** Every flattened style currently on screen, so a test can assert that some node does (or does not) carry one. */ -const getRenderedStyles = (): unknown[] => { - const renderedStyles: unknown[] = []; - for (const node of screen.UNSAFE_root.findAll(() => true)) { - renderedStyles.push(StyleSheet.flatten(node.props.style)); - } - return renderedStyles; -}; - -// `getTextInputIconContainerStyles` gives every icon container a full-height centred box, and `pr0` then strips the -// right half of its padding. That one-sided padding identifies the caret container. -const caretContainer = {height: '100%', paddingHorizontal: 11, paddingRight: 0}; - -const buildFieldList = (fieldCount: number): Record => { - const fieldList: Record = {}; - for (let index = 1; index <= fieldCount; index++) { - const field = buildTextField(index); - fieldList[`expensify_${field.fieldID}`] = field; - } - return fieldList; -}; - -const buildPolicy = (fieldCount: number, extraFields: OnyxTypes.PolicyReportField[] = []): OnyxTypes.Policy => { - const fieldList = buildFieldList(fieldCount); - for (const field of extraFields) { - fieldList[`expensify_${field.fieldID}`] = field; - } - - return { - ...LHNTestUtils.getFakePolicy(policyID, 'Policy'), - type: CONST.POLICY.TYPE.TEAM, - role: CONST.POLICY.ROLE.ADMIN, - outputCurrency: CONST.CURRENCY.USD, - areReportFieldsEnabled: true, - fieldList, - }; -}; - -const buildReport = (reportOnlyFields: OnyxTypes.PolicyReportField[] = []): OnyxTypes.Report => ({ - ...LHNTestUtils.getFakeReport([accountID, 2]), - fieldList: Object.fromEntries(reportOnlyFields.map((field) => [`expensify_${field.fieldID}`, field])), - reportID, - type: CONST.REPORT.TYPE.EXPENSE, - policyID, - ownerAccountID: accountID, - currency: CONST.CURRENCY.USD, - stateNum: CONST.REPORT.STATE_NUM.OPEN, - statusNum: CONST.REPORT.STATUS_NUM.OPEN, - total: 0, -}); - -const renderReportFields = async (fieldCount: number, extraFields: OnyxTypes.PolicyReportField[] = [], reportOnlyFields: OnyxTypes.PolicyReportField[] = []) => { - const policy = buildPolicy(fieldCount, extraFields); - const report = buildReport(reportOnlyFields); - - await act(async () => { - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy); - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, report); - }); - await waitForBatchedUpdatesWithAct(); - - const rendered = render( - - - , - ); - await waitForBatchedUpdatesWithAct(); - - return rendered; -}; - -describe('MoneyRequestViewReportFields', () => { - beforeAll(async () => { - Onyx.init({keys: ONYXKEYS, evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS]}); - initOnyxDerivedValues(); - // Field violation copy comes from `translateLocal`, not the mocked `useLocalize`, so the locale has to be real. - IntlStore.load('en'); - await waitForBatchedUpdatesWithAct(); - }); - - beforeEach(async () => { - mockShouldUseNarrowLayout = false; - jest.clearAllMocks(); - await act(async () => { - await TestHelper.signInWithTestUser(accountID); - }); - await waitForBatchedUpdatesWithAct(); - }); - - afterEach(async () => { - await act(async () => { - await Onyx.clear(); - }); - }); - - it('shows up to three fields per row on a wide layout', async () => { - // Given a report with 7 fields, a count that does not divide evenly into rows of three, because a partly - // filled last row is the case that would regress if the chunking were off by one - await renderReportFields(7); - - // When the fields are laid out on a wide layout - // Then they occupy 3 rows, two full rows of three plus one holding the remaining field, which is what packing - // the fields three to a row rather than stacking them produces - expect(screen.getAllByTestId('reportFieldsRow')).toHaveLength(3); - }); - - it('shows one field per row on a narrow layout', async () => { - // Given a narrow layout, where three fields side by side would each be too cramped to read - mockShouldUseNarrowLayout = true; - - // When 4 fields are laid out - await renderReportFields(4); - - // Then each one gets its own row, so the column count drops to 1 rather than staying at 3 - expect(screen.getAllByTestId('reportFieldsRow')).toHaveLength(4); - }); - - it('renders every field as an input holding its value', async () => { - // Given a report with 2 fields that already hold values - await renderReportFields(2); - - // When each field is looked up by its label - // Then it is an input carrying the saved value, because the fields are edited in place now instead of being - // rows that show the value and navigate to the editor page to change it - expect(screen.getByLabelText('Field1')).toHaveProp('value', 'Value1'); - expect(screen.getByLabelText('Field2')).toHaveProp('value', 'Value2'); - }); - - it('saves a value typed into a field without leaving the report', async () => { - // Given a report with 2 fields - await renderReportFields(2); - - // When a new value is typed into the first field and the field is left - const input = screen.getByLabelText('Field1'); - fireEvent.changeText(input, 'Updated value'); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then the value is saved from the report itself, with the old value passed along so the optimistic update can - // be rolled back. This is the whole point of inline editing: no navigation to the editor page to change a value - expect(updateReportField).toHaveBeenCalledTimes(1); - expect(jest.mocked(updateReportField).mock.calls.at(0)?.at(0)).toMatchObject({ - reportField: {fieldID: 'field1', value: 'Updated value'}, - previousReportField: {fieldID: 'field1', value: 'Value1'}, - }); - }); - - it('saves once when Enter is pressed and the field then loses focus', async () => { - // Given a report with 2 fields - await renderReportFields(2); - - // When a new value is typed and submitted with Enter, which on iOS and Android also blurs the input - const input = screen.getByLabelText('Field1'); - fireEvent.changeText(input, 'Updated value'); - fireEvent(input, 'submitEditing'); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then the value is saved only once, because each save posts its own system message to the report - expect(updateReportField).toHaveBeenCalledTimes(1); - expect(jest.mocked(updateReportField).mock.calls.at(0)?.at(0)).toMatchObject({ - reportField: {fieldID: 'field1', value: 'Updated value'}, - }); - }); - - it('saves an empty value when a text field is cleared', async () => { - // Given a report with 2 text fields that already hold values - await renderReportFields(2); - - // When the first field is emptied and left - const input = screen.getByLabelText('Field1'); - fireEvent.changeText(input, ''); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then the empty value is saved, because OldDot lets a text field be cleared and the report history then shows - // `removed Field1 (previously "Value1")` - expect(updateReportField).toHaveBeenCalledTimes(1); - expect(jest.mocked(updateReportField).mock.calls.at(0)?.at(0)).toMatchObject({ - reportField: {fieldID: 'field1', value: ''}, - previousReportField: {fieldID: 'field1', value: 'Value1'}, - }); - }); - - it('saves an empty value rather than whitespace when a text field is left holding only spaces', async () => { - // Given a report with 2 text fields that already hold values - await renderReportFields(2); - - // When the first field is replaced with spaces and left - const input = screen.getByLabelText('Field1'); - fireEvent.changeText(input, ' '); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then an empty value is saved, so the report never stores a value that looks empty but is not - expect(updateReportField).toHaveBeenCalledTimes(1); - expect(jest.mocked(updateReportField).mock.calls.at(0)?.at(0)).toMatchObject({ - reportField: {fieldID: 'field1', value: ''}, - }); - }); - - it('does not save when the option a list field already holds is picked again', async () => { - // Given a list field already set to Option1 - await renderReportFields(1, [buildListField()]); - - // When the dropdown is opened and that same option is picked - fireEvent.press(screen.getByLabelText('ListField')); - await waitForBatchedUpdatesWithAct(); - - fireEvent.press(screen.getByTestId('base-list-item-Option1')); - await waitForBatchedUpdatesWithAct(); - - // Then no update is sent, because a re-pick is not an edit and saving it would put a pointless entry in the - // report history and mark the field as recently used for a value the user never actually changed - expect(updateReportField).not.toHaveBeenCalled(); - }); - - it('does not save when the value is left unchanged', async () => { - // Given a report with 2 fields that already hold values - await renderReportFields(2); - - // When a field is focused and left without typing, which happens constantly as the user tabs through a row - const input = screen.getByLabelText('Field1'); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then nothing is saved, because blur is the save trigger now and merely passing through a field must not - // cost a write - expect(updateReportField).not.toHaveBeenCalled(); - }); - - it('edits the raw value of a text field whose default refers to another field', async () => { - // Given a text field with no value of its own whose default refers to Field1, so the report would display it - // computed as "Value1 code" - await renderReportFields(1, [{...buildTextField(2), name: 'LinkedField', fieldID: 'linkedField', defaultValue: '{field:Field1} code', value: null}]); - - // When the field is inspected - // Then the input holds the raw default, the same as the editor page, so editing it never saves the computed - // text over the reference and the field keeps following Field1 - expect(screen.getByLabelText('LinkedField')).toHaveProp('value', '{field:Field1} code'); - }); - - it('resets the displayed text when an edit is skipped because it only changed the whitespace', async () => { - // Given a report with 2 fields that already hold values - await renderReportFields(2); - - // When the value is retyped with surrounding whitespace and the field is left - const input = screen.getByLabelText('Field1'); - fireEvent.changeText(input, ' Value1 '); - fireEvent(input, 'blur'); - await waitForBatchedUpdatesWithAct(); - - // Then no update is sent, because the value is unchanged once trimmed - expect(updateReportField).not.toHaveBeenCalled(); - - // Then the field shows the saved value again rather than the whitespace the user typed. The saved value never - // changed, so nothing else would ever correct the display and the text would silently snap back on remount - expect(screen.getByLabelText('Field1')).toHaveProp('value', 'Value1'); - }); - - it('keeps the fields of a row aligned at the top so an error grows the row downwards', async () => { - // Given a full row of 3 fields - await renderReportFields(3); - - // When the row is laid out - // Then its fields are aligned to the top, so one field growing taller to show a validation error pushes its - // own content down instead of re-centering the two fields beside it - expect(screen.getAllByTestId('reportFieldsRow').at(0)).toHaveStyle({alignItems: 'flex-start'}); - }); - - it('shows a field violation as soon as the report opens', async () => { - // Given a required field that is already empty when the report opens, so its violation is true from the start - await renderReportFields(1, [buildEmptyField()]); - - // When the report is shown, before the user has touched the field - // Then the error is already visible, like the report field rows before inline editing, so a report with empty - // required fields tells the user what still needs filling in - expect(screen.getByText('EmptyField is required')).toBeOnTheScreen(); - }); - - it('renders a list field as a collapsed combobox rather than a row that opens a page', async () => { - // Given a report holding a list field set to Option1 - await renderReportFields(1, [buildListField()]); - - // When the field is inspected before it has been opened - const listInput = screen.getByLabelText('ListField'); - - // Then it exposes itself to assistive tech as a closed combobox showing the current value, which is what a - // dropdown that opens in place must look like, unlike the old row that announced itself as a link to a page - expect(listInput).toHaveProp('role', CONST.ROLE.COMBOBOX); - expect(listInput).toHaveProp('accessibilityState', {expanded: false}); - expect(listInput).toHaveProp('value', 'Option1'); - - // Then no option row is mounted yet, because building the option list for every list field on a report full of - // them would be paid for up front even though most dropdowns are never opened - expect(screen.queryByText('Option2')).toBeNull(); - }); - - it('hides the text cursor on a list field but keeps it on a text field', async () => { - // Given a report holding both a text field and a list field - await renderReportFields(1, [buildListField()]); - - // When the two inputs are compared - // Then the list field hides the cursor, because its value is picked from the dropdown rather than typed, and - // `disableKeyboard` alone only suppresses the software keyboard and would still leave a cursor blinking in a - // field the user cannot type into. - expect(screen.getByLabelText('ListField')).toHaveProp('caretHidden', true); - - // Then the text field is untouched and still shows its cursor. - expect(screen.getByLabelText('Field1')).not.toHaveProp('caretHidden', true); - }); - - it('opens the option list in place when a list field is pressed', async () => { - // Given a report holding a list field - await renderReportFields(1, [buildListField()]); - - // When the field is pressed - fireEvent.press(screen.getByLabelText('ListField')); - await waitForBatchedUpdatesWithAct(); - - // Then the options appear over the report and the combobox reports itself as expanded, which is what - // replacing the navigation to the editor page with an in-place dropdown has to produce - expect(screen.getByLabelText('ListField')).toHaveProp('accessibilityState', {expanded: true}); - expect(screen.getByText('Option2')).toBeOnTheScreen(); - }); - - it('keeps the option list open when one tap reaches the list field as both a press and a press out', async () => { - // Given a report holding a list field - await renderReportFields(1, [buildListField()]); - const listField = screen.getByLabelText('ListField'); - const tapEvent = {nativeEvent: {}}; - - // When a single tap reaches the field twice with the same event, the way the Android input reports it - // through both `onPressOut` and `onPress` - fireEvent(listField, 'pressOut', tapEvent); - fireEvent.press(listField, tapEvent); - await waitForBatchedUpdatesWithAct(); - - // Then the list stays open. The dropdown trigger toggles, so if the repeated report were not ignored the - // second call would close the list the first one opened, which is the list auto-closing on Android - expect(listField).toHaveProp('accessibilityState', {expanded: true}); - expect(screen.getByText('Option2')).toBeOnTheScreen(); - }); - - it('keeps the caret in the same place when the option list opens', async () => { - // Given a closed list field whose caret sits in a container padded on one side only - await renderReportFields(1, [buildListField()]); - - expect(getRenderedStyles()).toEqual(expect.arrayContaining([expect.objectContaining(caretContainer)])); - - // When the list is opened and the caret flips to point up - fireEvent.press(screen.getByLabelText('ListField')); - await waitForBatchedUpdatesWithAct(); - - // Then the caret is mirrored on the vertical axis rather than rotated. `rotate(180deg)` would carry the - // asymmetric padding around with it and slide the caret 11px left the instant the list opens, so the caret - // would visibly jump sideways every time the user opened a dropdown - const expandedStyles = getRenderedStyles(); - expect(expandedStyles).toEqual(expect.arrayContaining([expect.objectContaining({...caretContainer, transform: [{scaleY: -1}]})])); - expect(expandedStyles).not.toEqual(expect.arrayContaining([expect.objectContaining({transform: [{rotate: '180deg'}]})])); - }); - - it('does not pin the label of text and list fields in the raised position', async () => { - // Given a report holding both a text field and a list field - await renderReportFields(1, [buildListField()]); - - // When the inputs are inspected - const raisedLabels: unknown[] = []; - for (const node of screen.UNSAFE_root.findAll((candidate) => candidate.props.forceActiveLabel === true)) { - raisedLabels.push(node.props.label); - } - - // Then neither label is pinned, so an empty field shows its label inside the input like a placeholder and - // reads as empty instead of looking like a filled field - expect(raisedLabels).not.toEqual(expect.arrayContaining(['Field1'])); - expect(raisedLabels).not.toEqual(expect.arrayContaining(['ListField'])); - }); - - it('does not carry an unsaved edit into the next report', async () => { - // Given a report whose field holds an edit the user has typed but not saved yet - const {rerender} = await renderReportFields(1); - fireEvent.changeText(screen.getByLabelText('Field1'), 'Unsaved edit'); - await waitForBatchedUpdatesWithAct(); - - // When the next report arrow swaps in another report on the same workspace without remounting the screen - const policy = buildPolicy(1); - const nextReport = {...buildReport(), reportID: 'next_report_report_fields_grid'}; - rerender( - - - , - ); - await waitForBatchedUpdatesWithAct(); - - // Then the field shows the next report's value, because the edit belonged to the previous report and the - // field key alone is the same on every report of the workspace - expect(screen.getByLabelText('Field1')).toHaveProp('value', 'Value1'); - }); - - it('sizes the option popover like the Spend dropdowns instead of matching the field width', async () => { - // Given a report holding a list field, which in the grid can be as narrow as a third of a row - await renderReportFields(1, [buildListField()]); - - // When its option list is opened - fireEvent.press(screen.getByLabelText('ListField')); - await waitForBatchedUpdatesWithAct(); - - // Then the popover takes the standard dropdown width rather than the width of the field it is anchored to, - // because a popover that narrow left the option labels cramped and truncated - const popover = screen.UNSAFE_getByType(PopoverWithMeasuredContent); - expect(popover.props.popoverDimensions).toEqual(expect.objectContaining({width: CONST.POPOVER_DROPDOWN_WIDTH})); - }); - - it('shows a delete button only on a field that was deleted from the workspace', async () => { - // Given a report holding one field that is still on the workspace and one the admin has since deleted. Both - // are `deletable`, so the button has to be decided by the field being gone from the workspace - await renderReportFields(1, [], [buildDeletedField()]); - - // When the fields are rendered - // Then the deleted field still shows its value and carries a delete button, because the editor page with its - // Delete option is no longer reachable and this is the only way to clear the stale value off the report - expect(screen.getByLabelText('DeletedField')).toHaveProp('value', 'Old value'); - expect(screen.getByLabelText('workspace.reportFields.delete, DeletedField')).toBeOnTheScreen(); - - // And the field that is still on the workspace has no delete button, since removing it would only bring it back - expect(screen.queryByLabelText('workspace.reportFields.delete, Field1')).not.toBeOnTheScreen(); - }); - - it('shows a delete button on a deleted date field even when its copy on the report is not deletable', async () => { - // Given a report holding a date field the admin has since deleted from the workspace, whose copy on the report - // is flagged as not deletable - await renderReportFields( - 1, - [], - [{...buildDeletedField(), name: 'DeletedDateField', fieldID: 'deletedDateField', type: CONST.REPORT_FIELD_TYPES.DATE, value: '2026-10-01', deletable: false}], - ); - - // When the fields are rendered - // Then it still carries a delete button, because the field is gone from the workspace and there is no other - // way to clear its stale value off the report - expect(screen.getByLabelText('workspace.reportFields.delete, DeletedDateField')).toBeOnTheScreen(); - }); - - it('deletes the field from the report once the delete is confirmed', async () => { - // Given a report holding a field that was deleted from the workspace - await renderReportFields(1, [], [buildDeletedField()]); - - // When its delete button is pressed and the confirmation is accepted - fireEvent.press(screen.getByLabelText('workspace.reportFields.delete, DeletedField')); - await waitForBatchedUpdatesWithAct(); - - // Then the user is asked first, as on the editor page, and the field is removed from this report - expect(mockShowConfirmModal).toHaveBeenCalledTimes(1); - expect(deleteReportField).toHaveBeenCalledTimes(1); - expect(deleteReportField).toHaveBeenCalledWith(reportID, expect.objectContaining({fieldID: 'deletedField'})); - }); - - it('keeps the field when the delete is cancelled', async () => { - // Given a report holding a field that was deleted from the workspace - await renderReportFields(1, [], [buildDeletedField()]); - - // When its delete button is pressed but the confirmation is dismissed - mockShowConfirmModal.mockImplementationOnce(() => Promise.resolve({action: 'CLOSE'})); - fireEvent.press(screen.getByLabelText('workspace.reportFields.delete, DeletedField')); - await waitForBatchedUpdatesWithAct(); - - // Then nothing is deleted, so a stray tap on the button cannot wipe the value - expect(deleteReportField).not.toHaveBeenCalled(); - }); -}); diff --git a/tests/unit/ReportActionsUtilsTest.ts b/tests/unit/ReportActionsUtilsTest.ts index 69d8a0df33a8..dc4358e42198 100644 --- a/tests/unit/ReportActionsUtilsTest.ts +++ b/tests/unit/ReportActionsUtilsTest.ts @@ -1851,54 +1851,6 @@ describe('ReportActionsUtils', () => { }); describe('getMessageOfOldDotReportAction', () => { - it('should return the removed field message when a field with a value is cleared', () => { - // Given a field change whose new value is empty while the old value was set, which is what clearing a text field sends - const action: Parameters[1] = { - reportActionID: '1', - created: '2024-01-01 00:00:00.000', - actionName: CONST.REPORT.ACTIONS.TYPE.CHANGE_FIELD, - originalMessage: {fieldName: 'Project', oldValue: 'Apollo', newValue: ''}, - }; - - // When the message is built - const message = ReportActionsUtils.getMessageOfOldDotReportAction(translateLocal, action); - - // Then it says the field was removed and shows the old value, instead of `changed Project to ""` - expect(message).toBe('removed Project (previously "Apollo")'); - }); - - it('should return the set field message when a field without a value is filled in', () => { - // Given a field change whose old value is empty - const action: Parameters[1] = { - reportActionID: '1', - created: '2024-01-01 00:00:00.000', - actionName: CONST.REPORT.ACTIONS.TYPE.CHANGE_FIELD, - originalMessage: {fieldName: 'Project', oldValue: '', newValue: 'Apollo'}, - }; - - // When the message is built - const message = ReportActionsUtils.getMessageOfOldDotReportAction(translateLocal, action); - - // Then it says the field was set, because there is no previous value to show - expect(message).toBe('set Project to "Apollo"'); - }); - - it('should return the changed field message when a field value is replaced', () => { - // Given a field change where both the old and the new value are set - const action: Parameters[1] = { - reportActionID: '1', - created: '2024-01-01 00:00:00.000', - actionName: CONST.REPORT.ACTIONS.TYPE.CHANGE_FIELD, - originalMessage: {fieldName: 'Project', oldValue: 'Apollo', newValue: 'Gemini'}, - }; - - // When the message is built - const message = ReportActionsUtils.getMessageOfOldDotReportAction(translateLocal, action); - - // Then it shows both the new and the previous value - expect(message).toBe('changed Project to "Gemini" (previously "Apollo")'); - }); - it('should return the ACH bounce message with return reason when provided', () => { const returnReason = 'R03 - No Account/Unable to Locate Account'; const action: Parameters[1] = { diff --git a/tests/unit/ReportFieldOptionsListUtilsTest.ts b/tests/unit/ReportFieldOptionsListUtilsTest.ts index b1a5827f32f0..04ad47cc42cf 100644 --- a/tests/unit/ReportFieldOptionsListUtilsTest.ts +++ b/tests/unit/ReportFieldOptionsListUtilsTest.ts @@ -20,54 +20,43 @@ describe('ReportFieldOptionsListUtils', () => { describe('getReportFieldOptionsSection', () => { it('pins the frozen initial value to its own "Selected" section on top and excludes it from the "All" section', () => { - // Given a list opened on "Charlie", with the live selection still matching the value it opened on - // When the sections are built const sections = getReportFieldOptionsSection({ options: OPTIONS, recentlyUsedOptions: [], + // Live selection matches the frozen one on open. selectedOptions: [{text: 'Charlie'}], initiallySelectedValue: 'Charlie', searchValue: '', translate: translateLocal, }); - // Then "Charlie" is pinned to the top in its own checked section, so the user can see what the field - // currently holds without scrolling a long option list to find it const selectedSection = sectionByIndex(sections, 1); expect(selectedSection?.data.map((option) => option.keyForList)).toEqual(['Charlie']); expect(selectedSection?.data.at(0)?.isSelected).toBe(true); - - // Then it is not repeated in the "All" section, because the same option listed twice reads as two - // different options the user has to tell apart + // The pinned value must not be duplicated in the "All" section. expect(sectionByIndex(sections, 3)?.data.map((option) => option.keyForList)).not.toContain('Charlie'); }); it('keeps the frozen value pinned while the live selection drives the checkmark', () => { - // Given a list that opened on "Charlie" and on which the user has since picked "Delta" - // When the sections are rebuilt const sections = getReportFieldOptionsSection({ options: OPTIONS, recentlyUsedOptions: [], + // The user picked "Delta" (live), but the list opened on "Charlie" (frozen). selectedOptions: [{text: 'Delta'}], initiallySelectedValue: 'Charlie', searchValue: '', translate: translateLocal, }); - // Then "Charlie" stays pinned but loses its checkmark. Pinning on the live selection instead would make - // the option the user just clicked jump to the top of the list under their cursor const selectedSection = sectionByIndex(sections, 1); + // "Charlie" stays pinned (frozen) but is no longer checked... expect(selectedSection?.data.map((option) => option.keyForList)).toEqual(['Charlie']); expect(selectedSection?.data.at(0)?.isSelected).toBe(false); - - // Then the checkmark follows the live selection to where "Delta" naturally sits, so the list still shows - // the user what they picked + // ...and the live selection "Delta" is checked where it sits in the "All" section. expect(sectionByIndex(sections, 3)?.data.find((option) => option.keyForList === 'Delta')?.isSelected).toBe(true); }); it('keeps the pinned value at the top of the search results when it matches', () => { - // Given a list pinned to "Rate B" and a search term that every option matches - // When the sections are built const sections = getReportFieldOptionsSection({ options: ['Rate A', 'Rate B', 'Rate C'], recentlyUsedOptions: [], @@ -77,62 +66,14 @@ describe('ReportFieldOptionsListUtils', () => { translate: translateLocal, }); - // Then searching collapses everything into one section but the pinned value still leads it, so the - // current value does not disappear into the middle of the results the moment the user types + // Search collapses to a single section; the pinned value leads regardless of its natural position. const searchData = sectionByIndex(sections, 0)?.data.map((option) => option.keyForList) ?? []; expect(searchData.at(0)).toBe('Rate B'); - - // Then every matching option is still offered exactly once, because pinning reorders the results rather - // than adding a copy of the current value to them expect(searchData).toEqual(expect.arrayContaining(['Rate A', 'Rate B', 'Rate C'])); expect(searchData).toHaveLength(3); }); - it('leaves the selected value in place when pinning is turned off', () => { - // Given a caller that opts out of pinning, which the inline field does for a short option list - // When the sections are built - const sections = getReportFieldOptionsSection({ - options: OPTIONS, - recentlyUsedOptions: [], - selectedOptions: [{text: 'Charlie'}], - initiallySelectedValue: 'Charlie', - searchValue: '', - translate: translateLocal, - shouldPinSelectedOption: false, - }); - - // Then there is no "Selected" section and the options keep their natural order, because a handful of - // options is easier to scan in a stable order than one that moves the current value to the top - expect(sectionByIndex(sections, 1)).toBeUndefined(); - expect(sectionByIndex(sections, 3)?.data.map((option) => option.keyForList)).toEqual(OPTIONS); - - // Then the current value is still checked, so turning off pinning changes the ordering only and does not - // cost the user the ability to see what the field holds - expect(sectionByIndex(sections, 3)?.data.find((option) => option.keyForList === 'Charlie')?.isSelected).toBe(true); - }); - - it('drops the section titles when they are turned off', () => { - // Given a caller that opts out of section titles and data that would otherwise produce two titled - // sections, "Recent" and "All" - // When the sections are built - const sections = getReportFieldOptionsSection({ - options: OPTIONS, - recentlyUsedOptions: ['Delta'], - selectedOptions: [], - initiallySelectedValue: '', - searchValue: '', - translate: translateLocal, - shouldShowSectionTitles: false, - }); - - // Then both sections are still built but neither carries a title, so a small popover is not made to spend - // its limited height on headers while the options themselves stay grouped - expect(sections.map((section) => section.title)).toEqual(['', '']); - }); - it('does not force-show the pinned value when it does not match the search', () => { - // Given a list pinned to "Rate B" and a search term that only "Zulu" matches - // When the sections are built const sections = getReportFieldOptionsSection({ options: ['Rate A', 'Rate B', 'Zulu'], recentlyUsedOptions: [], @@ -142,8 +83,6 @@ describe('ReportFieldOptionsListUtils', () => { translate: translateLocal, }); - // Then only "Zulu" is offered. Pinning decides where a matching value sits, so it must not smuggle a - // non-matching option into results the user has explicitly filtered down expect(sectionByIndex(sections, 0)?.data.map((option) => option.keyForList)).toEqual(['Zulu']); }); });