diff --git a/contributingGuides/FORMS.md b/contributingGuides/FORMS.md index 464b6eba678b..0880ce415641 100644 --- a/contributingGuides/FORMS.md +++ b/contributingGuides/FORMS.md @@ -385,6 +385,33 @@ import KEYBOARD_SUBMIT_BEHAVIOR from './keyboardSubmitBehavior'; > [!NOTE] > Only override `keyboardSubmitBehavior` on screens where `onSubmit` triggers navigation. For forms that stay on-screen after submission, keep the default (`'dismiss-then-submit'`) to avoid layout jumps. +### Reacting to form value changes (`FormValueWatcher`) + +Sometimes a parent screen needs to react when a form value changes from outside the form — for example when an RHP picker writes to the Onyx draft via `setDraftValues` and the parent has to re-sync `FormProvider`'s internal state (clear `touched` flags, wipe local errors via `resetForm`, etc.). + +Instead of adding a `useEffect` inside the parent (which can't see `FormProvider`'s `inputValues` directly), use the `FormValueWatcher` primitive together with `FormProvider`'s children render prop. It's a render-null component that fires `onValuesChange(current, previous)` whenever the `values` reference changes, and short-circuits when the reference is identical (no spurious fires). + +```jsx + + {({inputValues}) => ( + <> + { + if (current.type === previous.type) { + return; + } + formRef.current?.resetForm(current); + }} + /> + {/* ...InputWrappers... */} + + )} + +``` + +Use it only when you genuinely need a cross-component reaction; for in-form logic, prefer `onValueChange` / `onInputChange` on the specific input. + ### Safe Area Padding Any `FormProvider.tsx` that has a button at the bottom. If the `` is inside a ``, the bottom safe area inset is handled automatically (`includeSafeAreaPaddingBottom` needs to be set to `true`, but its the default). diff --git a/src/ROUTES.ts b/src/ROUTES.ts index 13a0e2485a34..9c9c4c52658c 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -20,7 +20,7 @@ import {getUrlWithParams} from './libs/Url'; import SCREENS from './SCREENS'; import type {Screen} from './SCREENS'; import type {CompanyCardFeedWithDomainID, PersonalCardFeed} from './types/onyx'; -import type {ConnectionName, SageIntacctMappingName} from './types/onyx/Policy'; +import type {ConnectionName, PolicyReportFieldType, SageIntacctMappingName} from './types/onyx/Policy'; import type {CustomFieldType} from './types/onyx/PolicyEmployee'; type WorkspaceCompanyCardsAssignCardParams = { @@ -2613,6 +2613,10 @@ const ROUTES = { route: 'workspaces/:policyID/reports/listValues/:reportFieldID?', getRoute: (policyID: string, reportFieldID?: string) => `workspaces/${policyID}/reports/listValues/${reportFieldID ? encodeURIComponent(reportFieldID) : ''}` as const, }, + WORKSPACE_REPORT_FIELDS_TYPE_SELECTOR: { + route: 'workspaces/:policyID/reports/typeSelector/:currentType?', + getRoute: (policyID: string, currentType?: PolicyReportFieldType) => `workspaces/${policyID}/reports/typeSelector/${currentType ? encodeURIComponent(currentType) : ''}` as const, + }, WORKSPACE_REPORT_FIELDS_ADD_VALUE: { route: 'workspaces/:policyID/reports/addValue/:reportFieldID?', getRoute: (policyID: string, reportFieldID?: string) => `workspaces/${policyID}/reports/addValue/${reportFieldID ? encodeURIComponent(reportFieldID) : ''}` as const, diff --git a/src/SCREENS.ts b/src/SCREENS.ts index a0981dcb634d..779962652062 100644 --- a/src/SCREENS.ts +++ b/src/SCREENS.ts @@ -779,6 +779,7 @@ const SCREENS = { REPORT_FIELDS_VALUE_SETTINGS: 'Workspace_ReportFields_ValueSettings', REPORT_FIELDS_EDIT_VALUE: 'Workspace_ReportFields_EditValue', REPORT_FIELDS_EDIT_INITIAL_VALUE: 'Workspace_ReportFields_EditInitialValue', + REPORT_FIELDS_TYPE_SELECTOR: 'Workspace_ReportFields_TypeSelector', TAX_EDIT: 'Workspace_Tax_Edit', TAX_NAME: 'Workspace_Tax_Name', TAX_VALUE: 'Workspace_Tax_Value', diff --git a/src/components/Form/FormValueWatcher.tsx b/src/components/Form/FormValueWatcher.tsx new file mode 100644 index 000000000000..a156251698f4 --- /dev/null +++ b/src/components/Form/FormValueWatcher.tsx @@ -0,0 +1,38 @@ +import {useEffect, useRef} from 'react'; +import type {OnyxFormKey} from '@src/ONYXKEYS'; +import type {FormOnyxValues} from './types'; + +type FormValueWatcherProps = { + /** Form values to watch - the `inputValues` exposed by FormProvider's render prop. */ + values: FormOnyxValues; + + /** + * Fires when the `values` prop reference changes. + * Receives the new values and the previous values for direct comparison. + */ + onValuesChange: (current: FormOnyxValues, previous: FormOnyxValues) => void; +}; + +/** + * Render-null primitive that observes a `values` object and fires `onValuesChange` with `(current, previous)` + * whenever the reference changes. Opt-in: only consumers who render it pay the cost. + * + * Designed to live inside `FormProvider`'s render prop so consumers can react to draft-driven updates + * (e.g. from an RHP picker page) without having to wire up their own previous-value ref + effect. + */ +function FormValueWatcher({values, onValuesChange}: FormValueWatcherProps) { + const previousValuesRef = useRef(values); + useEffect(() => { + if (previousValuesRef.current === values) { + return; + } + const previous = previousValuesRef.current; + previousValuesRef.current = values; + onValuesChange(values, previous); + }, [values, onValuesChange]); + return null; +} + +FormValueWatcher.displayName = 'FormValueWatcher'; + +export default FormValueWatcher; diff --git a/src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx b/src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx index 21723ba7ae0b..2f627c52f914 100644 --- a/src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx +++ b/src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx @@ -906,6 +906,7 @@ const SettingsModalStackNavigator = createModalStackNavigator require('../../../../pages/workspace/reports/ReportFieldsAddListValuePage').default, [SCREENS.WORKSPACE.REPORT_FIELDS_VALUE_SETTINGS]: () => require('../../../../pages/workspace/reports/ReportFieldsValueSettingsPage').default, [SCREENS.WORKSPACE.REPORT_FIELDS_EDIT_INITIAL_VALUE]: () => require('../../../../pages/workspace/reports/ReportFieldsInitialValuePage').default, + [SCREENS.WORKSPACE.REPORT_FIELDS_TYPE_SELECTOR]: () => require('../../../../pages/workspace/reports/TypeSelector/TypeSelectorPage').default, [SCREENS.WORKSPACE.REPORT_FIELDS_EDIT_VALUE]: () => require('../../../../pages/workspace/reports/ReportFieldsEditValuePage').default, [SCREENS.WORKSPACE.ACCOUNTING.SAGE_INTACCT_IMPORT]: () => require('../../../../pages/workspace/accounting/intacct/import/SageIntacctImportPage').default, [SCREENS.WORKSPACE.ACCOUNTING.SAGE_INTACCT_TOGGLE_MAPPING]: () => diff --git a/src/libs/Navigation/linkingConfig/RELATIONS/WORKSPACE_TO_RHP.ts b/src/libs/Navigation/linkingConfig/RELATIONS/WORKSPACE_TO_RHP.ts index 9f6b443a4d85..4d985487e7b8 100755 --- a/src/libs/Navigation/linkingConfig/RELATIONS/WORKSPACE_TO_RHP.ts +++ b/src/libs/Navigation/linkingConfig/RELATIONS/WORKSPACE_TO_RHP.ts @@ -247,6 +247,7 @@ const WORKSPACE_TO_RHP: Partial['config'] = { [SCREENS.WORKSPACE.REPORT_FIELDS_EDIT_INITIAL_VALUE]: { path: ROUTES.WORKSPACE_EDIT_REPORT_FIELDS_INITIAL_VALUE.route, }, + [SCREENS.WORKSPACE.REPORT_FIELDS_TYPE_SELECTOR]: { + path: ROUTES.WORKSPACE_REPORT_FIELDS_TYPE_SELECTOR.route, + }, [SCREENS.CONNECT_EXISTING_BUSINESS_BANK_ACCOUNT_ROOT]: { path: ROUTES.BANK_ACCOUNT_CONNECT_EXISTING_BUSINESS_BANK_ACCOUNT.route, exact: true, diff --git a/src/libs/Navigation/types.ts b/src/libs/Navigation/types.ts index 93637a59c8b4..828432dbced2 100644 --- a/src/libs/Navigation/types.ts +++ b/src/libs/Navigation/types.ts @@ -15,7 +15,7 @@ import type NAVIGATORS from '@src/NAVIGATORS'; import type {Route as ExpensifyRoute, Route as Routes} from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import type {CompanyCardFeedWithDomainID, PersonalCardFeed} from '@src/types/onyx'; -import type {ConnectionName, SageIntacctMappingName} from '@src/types/onyx/Policy'; +import type {ConnectionName, PolicyReportFieldType, SageIntacctMappingName} from '@src/types/onyx/Policy'; import type {CustomFieldType} from '@src/types/onyx/PolicyEmployee'; import type {FileObject} from '@src/types/utils/Attachment'; import type {SIDEBAR_TO_SPLIT} from './linkingConfig/RELATIONS'; @@ -663,6 +663,10 @@ type SettingsNavigatorParamList = { policyID: string; reportFieldID: string; }; + [SCREENS.WORKSPACE.REPORT_FIELDS_TYPE_SELECTOR]: { + policyID: string; + currentType?: PolicyReportFieldType; + }; [SCREENS.WORKSPACE.MEMBER_DETAILS]: { policyID: string; accountID: string; diff --git a/src/pages/workspace/reports/CreateReportFieldsPage.tsx b/src/pages/workspace/reports/CreateReportFieldsPage.tsx index aaaec9b44d6c..d21e6ed9400d 100644 --- a/src/pages/workspace/reports/CreateReportFieldsPage.tsx +++ b/src/pages/workspace/reports/CreateReportFieldsPage.tsx @@ -2,6 +2,7 @@ import React, {useCallback, useEffect, useRef} from 'react'; import {View} from 'react-native'; import type {OnyxCollection} from 'react-native-onyx'; import FormProvider from '@components/Form/FormProvider'; +import FormValueWatcher from '@components/Form/FormValueWatcher'; import InputWrapper from '@components/Form/InputWrapper'; import type {FormInputErrors, FormOnyxValues, FormRef} from '@components/Form/types'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; @@ -11,7 +12,6 @@ import TextPicker from '@components/TextPicker'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import useThemeStyles from '@hooks/useThemeStyles'; -import DateUtils from '@libs/DateUtils'; import {addErrorMessage} from '@libs/ErrorUtils'; import {hasCircularReferences} from '@libs/Formula'; import Navigation from '@libs/Navigation/Navigation'; @@ -35,8 +35,6 @@ import TypeSelector from './TypeSelector'; type CreateReportFieldsPageProps = WithPolicyAndFullscreenLoadingProps & PlatformStackScreenProps; -const defaultDate = DateUtils.extractDate(new Date().toString()); - function WorkspaceCreateReportFieldsPage({ policy, route: { @@ -195,6 +193,18 @@ function WorkspaceCreateReportFieldsPage({ > {({inputValues}) => ( + { + if (previous[INPUT_IDS.TYPE] === undefined) { + return; + } + if (current[INPUT_IDS.TYPE] === previous[INPUT_IDS.TYPE]) { + return; + } + formRef.current?.resetForm(current); + }} + /> { - let initialValue; - if (type === CONST.REPORT_FIELD_TYPES.DATE) { - initialValue = defaultDate; - } else if (type === CONST.REPORT_FIELD_TYPES.FORMULA) { - initialValue = '{report:id}'; - } else { - initialValue = ''; - } - - formRef.current?.resetForm({ - ...inputValues, - type, - initialValue, - }); - }} + policyID={policyID} /> {inputValues[INPUT_IDS.TYPE] === CONST.REPORT_FIELD_TYPES.LIST && ( diff --git a/src/pages/workspace/reports/TypeSelector/TypeSelectorModal.tsx b/src/pages/workspace/reports/TypeSelector/TypeSelectorModal.tsx deleted file mode 100644 index c6080b7eee8a..000000000000 --- a/src/pages/workspace/reports/TypeSelector/TypeSelectorModal.tsx +++ /dev/null @@ -1,67 +0,0 @@ -import React from 'react'; -import {View} from 'react-native'; -import HeaderWithBackButton from '@components/HeaderWithBackButton'; -import Modal from '@components/Modal'; -import ScreenWrapper from '@components/ScreenWrapper'; -import Text from '@components/Text'; -import useThemeStyles from '@hooks/useThemeStyles'; -import type {ReportFieldItemType} from '@pages/workspace/reports/ReportFieldTypePicker'; -import ReportFieldTypePicker from '@pages/workspace/reports/ReportFieldTypePicker'; -import CONST from '@src/CONST'; -import type {PolicyReportFieldType} from '@src/types/onyx/Policy'; - -type TypeSelectorModalProps = { - /** Whether the modal is visible */ - isVisible: boolean; - - /** Selected type */ - currentType: PolicyReportFieldType; - - /** Label to display on field */ - label: string; - - /** Subtitle to display on field */ - subtitle: string; - - /** Function to call when the user selects a type */ - onTypeSelected: (value: ReportFieldItemType) => void; - - /** Function to call when the user closes the type selector modal */ - onClose: () => void; -}; - -function TypeSelectorModal({isVisible, currentType, label, subtitle, onTypeSelected, onClose}: TypeSelectorModalProps) { - const styles = useThemeStyles(); - - return ( - - - - - {subtitle} - - - - - ); -} - -export default TypeSelectorModal; diff --git a/src/pages/workspace/reports/TypeSelector/TypeSelectorPage.tsx b/src/pages/workspace/reports/TypeSelector/TypeSelectorPage.tsx new file mode 100644 index 000000000000..f9edd62c2e83 --- /dev/null +++ b/src/pages/workspace/reports/TypeSelector/TypeSelectorPage.tsx @@ -0,0 +1,80 @@ +import React from 'react'; +import {View} from 'react-native'; +import HeaderWithBackButton from '@components/HeaderWithBackButton'; +import ScreenWrapper from '@components/ScreenWrapper'; +import Text from '@components/Text'; +import useLocalize from '@hooks/useLocalize'; +import useOnyx from '@hooks/useOnyx'; +import useThemeStyles from '@hooks/useThemeStyles'; +import {setDraftValues} from '@libs/actions/FormActions'; +import DateUtils from '@libs/DateUtils'; +import Navigation from '@libs/Navigation/Navigation'; +import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; +import {hasAccountingConnections} from '@libs/PolicyUtils'; +import type {SettingsNavigatorParamList} from '@navigation/types'; +import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper'; +import type {ReportFieldItemType} from '@pages/workspace/reports/ReportFieldTypePicker'; +import ReportFieldTypePicker from '@pages/workspace/reports/ReportFieldTypePicker'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import ROUTES from '@src/ROUTES'; +import type SCREENS from '@src/SCREENS'; +import INPUT_IDS from '@src/types/form/WorkspaceReportFieldForm'; +import type {PolicyReportFieldType} from '@src/types/onyx/Policy'; + +type TypeSelectorPageProps = PlatformStackScreenProps; + +function getDefaultInitialValueForReportFieldType(type: PolicyReportFieldType): string { + if (type === CONST.REPORT_FIELD_TYPES.DATE) { + return DateUtils.extractDate(new Date().toString()); + } + if (type === CONST.REPORT_FIELD_TYPES.FORMULA) { + return '{report:id}'; + } + return ''; +} + +function TypeSelectorPage({ + route: { + params: {policyID, currentType}, + }, +}: TypeSelectorPageProps) { + const styles = useThemeStyles(); + const {translate} = useLocalize(); + const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); + + const onTypeSelected = (item: ReportFieldItemType) => { + setDraftValues(ONYXKEYS.FORMS.WORKSPACE_REPORT_FIELDS_FORM, { + [INPUT_IDS.TYPE]: item.value, + [INPUT_IDS.INITIAL_VALUE]: getDefaultInitialValueForReportFieldType(item.value), + }); + Navigation.goBack(ROUTES.WORKSPACE_CREATE_REPORT_FIELD.getRoute(policyID)); + }; + + return ( + + + + + {translate('workspace.reportFields.typeInputSubtitle')} + + + + + ); +} + +export default TypeSelectorPage; diff --git a/src/pages/workspace/reports/TypeSelector/index.tsx b/src/pages/workspace/reports/TypeSelector/index.tsx index 70a01e7bb0e7..1e29da202485 100644 --- a/src/pages/workspace/reports/TypeSelector/index.tsx +++ b/src/pages/workspace/reports/TypeSelector/index.tsx @@ -1,73 +1,46 @@ import {Str} from 'expensify-common'; import type {ForwardedRef} from 'react'; -import React, {useState} from 'react'; -import {View} from 'react-native'; +import React from 'react'; +import type {View} from 'react-native'; import type {MenuItemBaseProps} from '@components/MenuItem'; import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription'; import useLocalize from '@hooks/useLocalize'; +import Navigation from '@libs/Navigation/Navigation'; import {getReportFieldTypeTranslationKey} from '@libs/WorkspaceReportFieldUtils'; -import type {ReportFieldItemType} from '@pages/workspace/reports/ReportFieldTypePicker'; import CONST from '@src/CONST'; +import ROUTES from '@src/ROUTES'; import type {PolicyReportFieldType} from '@src/types/onyx/Policy'; -import TypeSelectorModal from './TypeSelectorModal'; type TypeSelectorProps = Pick & { /** Currently selected type */ value?: string; - /** Subtitle to display on field */ - subtitle?: string; - - /** Function to call when the user selects a type */ - onInputChange?: (value: string) => void; - - /** Function to call when the user selects a type */ - onTypeSelected?: (reportFieldType: PolicyReportFieldType) => void; + /** Policy ID used to build the picker route */ + policyID?: string; /** Reference to the outer element */ ref?: ForwardedRef; }; -function TypeSelector({value, label = '', rightLabel, subtitle = '', errorText = '', onInputChange, onTypeSelected, ref}: TypeSelectorProps) { +function TypeSelector({value, label = '', rightLabel, errorText = '', policyID, ref}: TypeSelectorProps) { const {translate} = useLocalize(); - const [isPickerVisible, setIsPickerVisible] = useState(false); - - const showPickerModal = () => { - setIsPickerVisible(true); - }; - - const hidePickerModal = () => { - setIsPickerVisible(false); - }; - - const updateTypeInput = (reportField: ReportFieldItemType) => { - onInputChange?.(reportField.value); - onTypeSelected?.(reportField.value); - hidePickerModal(); - }; - return ( - - - - + { + if (!policyID) { + return; + } + Navigation.navigate(ROUTES.WORKSPACE_REPORT_FIELDS_TYPE_SELECTOR.getRoute(policyID, value as PolicyReportFieldType | undefined)); + }} + /> ); } diff --git a/tests/unit/components/Form/FormValueWatcher.test.tsx b/tests/unit/components/Form/FormValueWatcher.test.tsx new file mode 100644 index 000000000000..ec427d75fe3d --- /dev/null +++ b/tests/unit/components/Form/FormValueWatcher.test.tsx @@ -0,0 +1,160 @@ +import {render} from '@testing-library/react-native'; +import React from 'react'; +import FormValueWatcher from '@components/Form/FormValueWatcher'; +import type {FormOnyxValues} from '@components/Form/types'; +import CONST from '@src/CONST'; +import type ONYXKEYS from '@src/ONYXKEYS'; + +type TestFormID = typeof ONYXKEYS.FORMS.WORKSPACE_REPORT_FIELDS_FORM; +type TestValues = FormOnyxValues; + +function makeValues(overrides: Partial = {}): TestValues { + return { + name: '', + type: CONST.REPORT_FIELD_TYPES.TEXT, + initialValue: '', + listValues: [], + disabledListValues: [], + valueName: '', + newValueName: '', + ...overrides, + }; +} + +describe('FormValueWatcher', () => { + it('renders nothing (returns null)', () => { + const onValuesChange = jest.fn(); + const {toJSON} = render( + + values={makeValues()} + onValuesChange={onValuesChange} + />, + ); + expect(toJSON()).toBeNull(); + }); + + it('does not fire on initial mount', () => { + const onValuesChange = jest.fn(); + render( + + values={makeValues()} + onValuesChange={onValuesChange} + />, + ); + expect(onValuesChange).not.toHaveBeenCalled(); + }); + + it('fires with (current, previous) when the values reference changes', () => { + const onValuesChange = jest.fn(); + const initial = makeValues({type: CONST.REPORT_FIELD_TYPES.TEXT}); + const next = makeValues({type: CONST.REPORT_FIELD_TYPES.DATE}); + const {rerender} = render( + + values={initial} + onValuesChange={onValuesChange} + />, + ); + rerender( + + values={next} + onValuesChange={onValuesChange} + />, + ); + expect(onValuesChange).toHaveBeenCalledTimes(1); + expect(onValuesChange).toHaveBeenCalledWith(next, initial); + }); + + it('does not fire when re-rendered with the same values reference', () => { + const onValuesChange = jest.fn(); + const initial = makeValues(); + const {rerender} = render( + + values={initial} + onValuesChange={onValuesChange} + />, + ); + rerender( + + values={initial} + onValuesChange={onValuesChange} + />, + ); + expect(onValuesChange).not.toHaveBeenCalled(); + }); + + it('passes the correct (current, previous) pairs across three updates', () => { + const onValuesChange = jest.fn(); + const v1 = makeValues({type: CONST.REPORT_FIELD_TYPES.TEXT}); + const v2 = makeValues({type: CONST.REPORT_FIELD_TYPES.DATE}); + const v3 = makeValues({type: CONST.REPORT_FIELD_TYPES.FORMULA}); + const {rerender} = render( + + values={v1} + onValuesChange={onValuesChange} + />, + ); + rerender( + + values={v2} + onValuesChange={onValuesChange} + />, + ); + rerender( + + values={v3} + onValuesChange={onValuesChange} + />, + ); + expect(onValuesChange).toHaveBeenCalledTimes(2); + expect(onValuesChange).toHaveBeenNthCalledWith(1, v2, v1); + expect(onValuesChange).toHaveBeenNthCalledWith(2, v3, v2); + }); + + it('does not fire when only the onValuesChange callback identity changes', () => { + const cb1 = jest.fn(); + const cb2 = jest.fn(); + const initial = makeValues(); + const {rerender} = render( + + values={initial} + onValuesChange={cb1} + />, + ); + rerender( + + values={initial} + onValuesChange={cb2} + />, + ); + expect(cb1).not.toHaveBeenCalled(); + expect(cb2).not.toHaveBeenCalled(); + }); + + it('uses the latest onValuesChange after a swap followed by a values change', () => { + const cb1 = jest.fn(); + const cb2 = jest.fn(); + const initial = makeValues({type: CONST.REPORT_FIELD_TYPES.TEXT}); + const next = makeValues({type: CONST.REPORT_FIELD_TYPES.DATE}); + const {rerender} = render( + + values={initial} + onValuesChange={cb1} + />, + ); + rerender( + + values={initial} + onValuesChange={cb2} + />, + ); + rerender( + + values={next} + onValuesChange={cb2} + />, + ); + expect(cb1).not.toHaveBeenCalled(); + expect(cb2).toHaveBeenCalledTimes(1); + expect(cb2).toHaveBeenCalledWith(next, initial); + }); +});