diff --git a/config/eslint/eslint.seatbelt.tsv b/config/eslint/eslint.seatbelt.tsv index 5fec781e7e52..944543dbab25 100644 --- a/config/eslint/eslint.seatbelt.tsv +++ b/config/eslint/eslint.seatbelt.tsv @@ -301,7 +301,7 @@ "../../src/components/ReimbursementAccountLoadingIndicator.tsx" "no-restricted-imports" 1 "../../src/components/ReportActionAvatars/useReportPreviewSenderID.ts" "@typescript-eslint/no-unsafe-type-assertion" 2 "../../src/components/ReportActionItem/IssueCardMessage.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 -"../../src/components/ReportActionItem/MoneyReportView.tsx" "@typescript-eslint/no-unsafe-type-assertion" 2 +"../../src/components/ReportActionItem/MoneyReportView.tsx" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../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/receiptHoverUtils/index.ts" "@typescript-eslint/no-unsafe-type-assertion" 2 @@ -958,7 +958,6 @@ "../../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" "no-restricted-imports" 1 "../../src/pages/DynamicNewReportWorkspaceSelectionPage.tsx" "no-restricted-imports" 1 diff --git a/src/CONST/index.ts b/src/CONST/index.ts index d8db94233e37..6d96b95615ad 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -7402,6 +7402,12 @@ 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, @@ -9709,6 +9715,7 @@ 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 c228d91bd00f..be80fde9d11f 100644 --- a/src/components/DatePicker/index.tsx +++ b/src/components/DatePicker/index.tsx @@ -49,9 +49,13 @@ function DatePicker({ forwardedFSClass, shouldDeferShowUntilPositioned = false, shouldDismissKeyboardBeforeShow = false, + wrapperStyle, + onBlur, rightHandSideComponent, onPickerVisibilityChange, shouldHideCalendarIcon = false, + shouldEnableMonthYearBackdropInNarrowPane = false, + textInputContainerStyles, }: DateInputWithPickerProps) { const icons = useMemoizedLazyExpensifyIcons(['Calendar']); const styles = useThemeStyles(); @@ -228,13 +232,15 @@ function DatePicker({ <> showDatePickerModal()} + onBlur={onBlur} onSubmitEditing={() => showDatePickerModal()} onKeyPress={handleInputKeyPress} - textInputContainerStyles={isModalVisible ? styles.borderColorFocus : {}} + textInputContainerStyles={[textInputContainerStyles, isModalVisible && styles.borderColorFocus]} shouldHideClearButton={shouldHideClearButton} onClearInput={handleClear} forwardedFSClass={forwardedFSClass} @@ -271,6 +278,7 @@ function DatePicker({ shouldPositionFromTop={!isInverted} forwardedFSClass={forwardedFSClass} shouldCloseWhenBrowserNavigationChanged + shouldEnableMonthYearBackdropInNarrowPane={shouldEnableMonthYearBackdropInNarrowPane} /> ); diff --git a/src/components/DatePicker/types.ts b/src/components/DatePicker/types.ts index 8f54da70932f..b501e0740afa 100644 --- a/src/components/DatePicker/types.ts +++ b/src/components/DatePicker/types.ts @@ -5,6 +5,8 @@ import type {ForwardedFSClassProps} from '@libs/Fullstory/types'; import type {OnyxFormValuesMapping} from '@src/ONYXKEYS'; +import type {StyleProp, ViewStyle} from 'react-native'; + type DatePickerBaseProps = ForwardedFSClassProps & { /** * The datepicker supports any value that `new Date()` can parse. @@ -70,6 +72,13 @@ 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 311681b30cfd..0752ad30b8a1 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx @@ -371,7 +371,8 @@ function MoneyRequestReportActionsListContent({reportIDFromRoute, onLayout}: Mon onViewableItemsChanged={onViewableItemsChanged} onEndReached={onEndReached} onStartReached={onStartReached} - contentContainerStyle={[shouldUseNarrowLayout ? styles.pt4 : styles.pt3, shouldReserveBulkActionBarSpace && styles.bulkActionBarListSpacing]} + // 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]} 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 729a87232a79..9f79693af7ed 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestReportEmptyStateView.tsx @@ -27,7 +27,8 @@ 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. + ; @@ -44,53 +47,99 @@ 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, styles: ThemeStyles, pendingAction?: PendingAction) { +function ReportFieldView( + reportField: EnrichedPolicyReportField, + report: OnyxEntry, + policy: OnyxEntry, + styles: ThemeStyles, + onSaveValue: (reportField: PolicyReportField, value: string) => void, + onDelete: (reportField: EnrichedPolicyReportField) => Promise, + pendingAction?: PendingAction, +) { return ( - clearReportFieldKeyErrors(report?.reportID, reportField.fieldKey)} + - { - if (!report?.policyID) { - return; + clearReportFieldKeyErrors(report?.reportID, reportField.fieldKey)} + > + onSaveValue(reportField, value)} + onDelete={ + reportField.isFieldDeletable + ? () => { + onDelete(reportField); + } + : undefined } - - 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}: MoneyRequestViewReportFieldsProps) { +function MoneyRequestViewReportFields({report, policy, pendingAction, style, shouldUseSingleColumn = false}: 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); @@ -105,6 +154,8 @@ function MoneyRequestViewReportFields({report, policy, pendingAction}: MoneyRequ 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; const violation = isFieldDisabled ? undefined : getFieldViolation(field); const violationTranslation = getFieldViolationTranslation(field, violation); @@ -112,7 +163,11 @@ function MoneyRequestViewReportFields({report, policy, pendingAction}: MoneyRequ return { ...field, fieldValue, - isFieldDisabled: isFieldDisabled && !isDeletedFormulaField, + 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, fieldKey, violation, violationTranslation, @@ -128,10 +183,35 @@ function MoneyRequestViewReportFields({report, policy, pendingAction}: MoneyRequ 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 ( - - {sortedPolicyReportFields.map((reportField) => { - return ReportFieldView(reportField, report, styles, pendingAction); + + {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) => ( + + ))} + + ); })} ); diff --git a/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx b/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx new file mode 100644 index 000000000000..fdfbe35818c8 --- /dev/null +++ b/src/components/MoneyRequestReportView/ReportFieldInlineInput.tsx @@ -0,0 +1,303 @@ +/** + * 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 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 blurred right after it was submitted, is skipped while the update 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); + + // 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 valueToSave = StringUtils.lineBreaksToSpaces(draftValue); + const trimmedValue = valueToSave.trim(); + + if (trimmedValue === lastSavedValue.trim()) { + 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 (trimmedValue === '') { + // Clearing a field is a real change, so the empty value is saved even when the field is required. Like + // Classic, a required field left empty is flagged by its violation rather than blocked from saving. Any + // whitespace the user left behind is normalized away first so the stored value is empty rather than blank. + setValidationError(''); + setDraftValue(''); + setLastSavedValue(''); + onSaveValue(''); + 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( + , + ); +} + +export default ReportFieldInlineInput; diff --git a/src/components/ReportActionItem/MoneyReportView.tsx b/src/components/ReportActionItem/MoneyReportView.tsx index 07d679851ffa..bfd03faed323 100644 --- a/src/components/ReportActionItem/MoneyReportView.tsx +++ b/src/components/ReportActionItem/MoneyReportView.tsx @@ -1,13 +1,11 @@ import ActivityIndicator from '@components/ActivityIndicator'; import Icon from '@components/Icon'; -import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription'; -import OfflineWithFeedback from '@components/OfflineWithFeedback'; +import MoneyRequestViewReportFields from '@components/MoneyRequestReportView/MoneyRequestViewReportFields'; 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'; @@ -17,22 +15,15 @@ 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, @@ -45,16 +36,13 @@ 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'; @@ -95,13 +83,11 @@ 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, getCurrencyDecimals} = useCurrencyListActions(); + const {convertToDisplayString} = useCurrencyListActions(); const {isOffline} = useNetwork(); - const [rules] = useOnyx(ONYXKEYS.COLLECTION.RULE); const isSettled = isSettledReportUtils(report?.reportID); const isTotalUpdated = hasUpdatedTotal(report, policy) && !isTotalPending; @@ -136,13 +122,12 @@ function MoneyReportView({ StyleUtils.getColorStyle(theme.textSupporting), ]; - 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]); + // Only used to decide whether the report field block is worth rendering. + // `MoneyRequestViewReportFields` builds and resolves the fields it displays itself. + const {fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); + const sortedPolicyReportFields = Object.values(fieldsByName) + .filter((field) => isReportFieldTargetMatchingReport(report, field)) + .sort(({orderWeight: a}, {orderWeight: b}) => a - b); const isOnlyTitleFieldEnabled = sortedPolicyReportFields.every(shouldHideSingleReportField); const isClosedExpenseReportWithNoExpenses = isClosedExpenseReportWithNoExpensesReportUtils(report); @@ -175,50 +160,18 @@ function MoneyReportView({ {!isClosedExpenseReportWithNoExpenses && ( <> {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} - /> - - ); - })} + (!isCombinedReport || !isOnlyTitleFieldEnabled) && ( + // One-expense reports edit their fields in place like the report view does, instead of + // opening the report field editor page. They stay one field per row though: this screen + // reads as a single expense rather than a table, so the fields stack vertically here. + + )} {shouldShowTotalRow && ( diff --git a/src/components/Search/FilterDropdowns/FilterPopupButton.tsx b/src/components/Search/FilterDropdowns/FilterPopupButton.tsx index 5bf8eff98616..351f81a43ee1 100644 --- a/src/components/Search/FilterDropdowns/FilterPopupButton.tsx +++ b/src/components/Search/FilterDropdowns/FilterPopupButton.tsx @@ -67,7 +67,11 @@ function FilterPopupButton({ const isFocused = useIsFocused(); const styles = useThemeStyles(); const {isKeyboardActive} = useKeyboardState(); - const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: isSmallScreenWidth && !isKeyboardActive}); + // 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 StyleUtils = useStyleUtils(); const {windowHeight} = useWindowDimensions(); const triggerRef = useRef | null>(null); diff --git a/src/hooks/useSaveReportField.ts b/src/hooks/useSaveReportField.ts new file mode 100644 index 000000000000..dc76ffa6a71d --- /dev/null +++ b/src/hooks/useSaveReportField.ts @@ -0,0 +1,79 @@ +/** + * 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; + } + + if (isReportFieldOfTypeTitle(reportField)) { + // A report always has a name, so clearing the title is discarded rather than saved. + if (value === '') { + return; + } + 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/libs/ReportFieldOptionsListUtils.ts b/src/libs/ReportFieldOptionsListUtils.ts index 610c1bdfe700..912280000169 100644 --- a/src/libs/ReportFieldOptionsListUtils.ts +++ b/src/libs/ReportFieldOptionsListUtils.ts @@ -30,6 +30,8 @@ function getReportFieldOptionsSection({ initiallySelectedValue, searchValue, translate, + shouldPinSelectedOption = true, + shouldShowSectionTitles = true, }: { options: string[]; recentlyUsedOptions: string[]; @@ -37,12 +39,18 @@ 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 = initiallySelectedValue ? [initiallySelectedValue] : []; + const pinnedOptionKeys = shouldPinSelectedOption && initiallySelectedValue ? [initiallySelectedValue] : []; const pinnedKeySet = new Set(pinnedOptionKeys); if (searchValue) { @@ -81,7 +89,7 @@ function getReportFieldOptionsSection({ if (filteredRecentlyUsedOptions.length > 0) { reportFieldOptionsSections.push({ // "Recent" section - title: translate('common.recent'), + title: shouldShowSectionTitles ? translate('common.recent') : '', sectionIndex: 2, data: getReportFieldOptions(filteredRecentlyUsedOptions, selectedKeySet), }); @@ -89,7 +97,7 @@ function getReportFieldOptionsSection({ reportFieldOptionsSections.push({ // "All" section when items amount more than the threshold - title: translate('common.all'), + title: shouldShowSectionTitles ? 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 feae9942d704..558f29b0fb4d 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: Policy; + policy: OnyxEntry; isASAPSubmitBetaEnabled: boolean; accountID: number; email: string; diff --git a/src/pages/DynamicEditReportFieldPage.tsx b/src/pages/DynamicEditReportFieldPage.tsx index 6a63a50e7a10..f24e212352c5 100644 --- a/src/pages/DynamicEditReportFieldPage.tsx +++ b/src/pages/DynamicEditReportFieldPage.tsx @@ -2,7 +2,6 @@ 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'; @@ -12,10 +11,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, updateReportField, updateReportName} from '@libs/actions/Report'; +import {deleteReportField} 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'; @@ -25,10 +24,8 @@ import { getReportFieldFromReportNameValuePairs, getReportFieldKey, getTitleFieldWithFallback, - hasViolations as hasViolationsReportUtils, isGroupPolicyExpenseReport, isInvoiceReport, - isReportFieldDisabled, isReportFieldDisabledForUser, isReportFieldOfTypeTitle, isReportFieldTargetMatchingReport, @@ -38,9 +35,7 @@ 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'; @@ -58,7 +53,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 [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); + const saveReportField = useSaveReportField(report, policy); const isTitleField = route.params.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID; const reportFieldFromNVP = getReportFieldFromReportNameValuePairs(reportNameValuePairs, fieldKey) ?? getReportFieldFromReportNameValuePairs(reportNameValuePairs, route.params.fieldID); @@ -76,20 +71,12 @@ 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 ( @@ -140,28 +127,8 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { return; } - 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(); - } + saveReportField(reportField, value); + goBack(); }; const menuItems: PopoverMenuItem[] = []; diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index 038d01ca64de..e658ece4d9df 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -5,6 +5,7 @@ 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'; @@ -25,13 +26,39 @@ 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}: EditReportFieldDropdownPageProps) { +function EditReportFieldDropdown({ + onSubmit, + fieldKey, + fieldValue, + fieldOptions, + shouldShowTextInput = true, + shouldShowRecentlyUsedOptions = true, + shouldPinSelectedOption = true, + shouldShowSectionTitles = true, + shouldUseCompactRows = false, +}: EditReportFieldDropdownPageProps) { + const styles = useThemeStyles(); const [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState(''); const {translate, localeCompare} = useLocalize(); - const recentlyUsedOptions = recentlyUsedReportFields?.[fieldKey]?.sort(localeCompare) ?? []; + const recentlyUsedOptions = shouldShowRecentlyUsedOptions ? (recentlyUsedReportFields?.[fieldKey]?.sort(localeCompare) ?? []) : []; const validFieldOptions = fieldOptions?.filter((option) => !!option)?.sort(localeCompare); @@ -39,7 +66,7 @@ function EditReportFieldDropdown({onSubmit, fieldKey, fieldValue, fieldOptions}: // 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 sections = getReportFieldOptionsSection({ + const optionSections = getReportFieldOptionsSection({ searchValue: debouncedSearchValue, // Live value drives the checkmark, so tapping a row marks it immediately. selectedOptions: [ @@ -54,8 +81,17 @@ function EditReportFieldDropdown({onSubmit, fieldKey, fieldValue, fieldOptions}: 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. + const sections = shouldUseCompactRows + ? optionSections.map((section) => ({...section, data: section.data.map((option) => ({...option, itemStyle: styles.optionRowCompactMinHeight}))})) + : optionSections; + const policyReportFieldData = sections.at(0)?.data ?? []; const textInputOptions = { @@ -63,15 +99,20 @@ function EditReportFieldDropdown({onSubmit, fieldKey, fieldValue, fieldOptions}: 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 || fieldValue === option.text ? '' : option.text})} + // Re-selecting the value the field already holds submits that same value rather than an empty string. + // Both call sites compare the submitted value against the stored one and skip the save themselves, so + // signalling "no change" this way is no longer needed and leaves '' free to mean "cleared". + onSelectRow={(option) => onSubmit({[fieldKey]: option?.text ?? ''})} initiallyFocusedItemKey={initialFieldValue} shouldUpdateFocusedIndex /> diff --git a/src/styles/index.ts b/src/styles/index.ts index 18f1d71e7919..d2a31b2e7935 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -2211,6 +2211,25 @@ 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, + height: variables.componentSizeLarge, + 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, @@ -3339,6 +3358,14 @@ 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 new file mode 100644 index 000000000000..9bc104cc7955 --- /dev/null +++ b/tests/ui/EditReportFieldDropdownTest.tsx @@ -0,0 +1,131 @@ +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 5d525c985783..a083c77c75fa 100644 --- a/tests/ui/MoneyReportViewTest.tsx +++ b/tests/ui/MoneyReportViewTest.tsx @@ -19,6 +19,20 @@ 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), @@ -342,6 +356,8 @@ describe('MoneyReportView report fields visibility', () => { }); it('keeps a custom report field visible for a non-admin submitter after the report is approved (single-expense combined view)', async () => { + // Given an approved report with a custom field, viewed by its non-admin submitter, who cannot edit a field + // once the report has been approved const fieldList = { [CONST.REPORT_FIELD_TITLE_FIELD_ID]: buildTitleField(), [customFieldKey]: buildCustomTextField(), @@ -354,12 +370,43 @@ describe('MoneyReportView report fields visibility', () => { }); await seedReportFieldsPolicy(policy, approvedReport); + // When the combined view is rendered renderMoneyReportView(approvedReport, policy, true); await waitForBatchedUpdatesWithAct(); - // The custom field (rendered read-only after approval) must still show for the submitter. + // Then the field is still on the screen, because a submitter who can no longer change a value still needs to + // read it. It is looked up by its label since it is a read-only inline input now rather than a row that opens + // the report field editor + await waitFor(() => { + expect(screen.getByLabelText('Test')).toBeOnTheScreen(); + }); + }); + + it('stacks the report fields one per row on a wide layout, because this is the one-expense report view', async () => { + // Given a one-expense report with 4 custom fields on a wide layout + 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; + } + const policy = buildReportFieldsPolicy(fieldList); + const report = buildExpenseReport({fieldList}); + await seedReportFieldsPolicy(policy, report); + + // When this view renders them + renderMoneyReportView(report, policy); + await waitForBatchedUpdatesWithAct(); + + // Then each field gets 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.getByText('Test')).toBeOnTheScreen(); + expect(screen.getAllByTestId('reportFieldsRow')).toHaveLength(4); }); }); diff --git a/tests/ui/MoneyRequestViewReportFieldsTest.tsx b/tests/ui/MoneyRequestViewReportFieldsTest.tsx new file mode 100644 index 000000000000..bd7cd54540ec --- /dev/null +++ b/tests/ui/MoneyRequestViewReportFieldsTest.tsx @@ -0,0 +1,612 @@ +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: '', +}); + +// A field that cannot be deleted must hold a value. Leaving it empty raises a " is required" violation. +const buildRequiredField = (): OnyxTypes.PolicyReportField => ({ + ...buildTextField(1), + name: 'RequiredField', + fieldID: 'requiredField', + deletable: false, +}); + +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[] = [], + shouldUseSingleColumn = false, + 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('shows one field per row on a wide layout when a single column is requested', async () => { + // Given a wide layout and a caller asking for a single column, which is how the one-expense report view + // renders its fields inside a container too narrow for the grid + // When 4 fields are laid out + await renderReportFields(4, [], true); + + // Then each one gets its own row, proving `shouldUseSingleColumn` overrides the wide layout rather than only + // taking effect when the layout is already narrow + 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 the empty value when an optional field is cleared', async () => { + // Given a report with 2 optional 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 deliberately clearing an optional field is a real edit and must not + // be mistaken for the no-op case where the user tabbed through without changing anything + 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 the empty value when a required field is cleared and keeps its violation visible', async () => { + // Given a report holding a required field + const rendered = await renderReportFields(1, [buildRequiredField()]); + + // When the field is emptied and left + const input = screen.getByLabelText('RequiredField'); + fireEvent.changeText(input, ''); + fireEvent(input, 'blur'); + await waitForBatchedUpdatesWithAct(); + + // Then the empty value is saved, because like Classic a required field left empty is flagged by a violation + // rather than blocked from saving + expect(updateReportField).toHaveBeenCalledTimes(1); + expect(jest.mocked(updateReportField).mock.calls.at(0)?.at(0)).toMatchObject({ + reportField: {fieldID: 'requiredField', value: ''}, + previousReportField: {fieldID: 'requiredField', value: 'Value1'}, + }); + expect(screen.queryByText('common.error.fieldRequired')).toBeNull(); + + // When the report comes back holding the empty value the input just saved + rendered.rerender( + + + , + ); + await waitForBatchedUpdatesWithAct(); + + // Then the required violation shows, so the empty value the save created is flagged instead of blocked + expect(screen.getByText('RequiredField is required')).toBeOnTheScreen(); + }); + + 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('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, [], false, [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, [], false, [ + {...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, [], false, [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, [], false, [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/ReportFieldOptionsListUtilsTest.ts b/tests/unit/ReportFieldOptionsListUtilsTest.ts index 04ad47cc42cf..b1a5827f32f0 100644 --- a/tests/unit/ReportFieldOptionsListUtilsTest.ts +++ b/tests/unit/ReportFieldOptionsListUtilsTest.ts @@ -20,43 +20,54 @@ 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); - // The pinned value must not be duplicated in the "All" section. + + // 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 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); - // ...and the live selection "Delta" is checked where it sits in the "All" section. + + // Then the checkmark follows the live selection to where "Delta" naturally sits, so the list still shows + // the user what they picked 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: [], @@ -66,14 +77,62 @@ describe('ReportFieldOptionsListUtils', () => { translate: translateLocal, }); - // Search collapses to a single section; the pinned value leads regardless of its natural position. + // 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 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: [], @@ -83,6 +142,8 @@ 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']); }); });