Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import useLocalize from '@hooks/useLocalize';
import navigationRef from '@libs/Navigation/navigationRef';
import type DiscardChangesConfirmationProps from './types';

function DiscardChangesConfirmation({getHasUnsavedChanges, isEnabled = true}: DiscardChangesConfirmationProps) {
function DiscardChangesConfirmation({getHasUnsavedChanges, onVisibilityChange, isEnabled = true}: DiscardChangesConfirmationProps) {
const {translate} = useLocalize();
const isFocused = useIsFocused();
const [isVisible, setIsVisible] = useState(false);
Expand All @@ -16,12 +16,23 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, isEnabled = true}: Di
const hasUnsavedChanges = isEnabled && isFocused && getHasUnsavedChanges();
const shouldPrevent = hasUnsavedChanges && !shouldAllowNavigation.current;

const setModalVisible = useCallback(
(nextVisible: boolean) => {
setIsVisible(nextVisible);
onVisibilityChange?.(nextVisible);
},
[onVisibilityChange],
);

usePreventRemove(
shouldPrevent,
useCallback(({data}) => {
blockedNavigationAction.current = data.action;
setIsVisible(true);
}, []),
useCallback(
({data}) => {
blockedNavigationAction.current = data.action;
setModalVisible(true);
},
[setModalVisible],
),
);

return (
Expand All @@ -33,7 +44,7 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, isEnabled = true}: Di
confirmText={translate('discardChangesConfirmation.confirmText')}
cancelText={translate('common.cancel')}
onConfirm={() => {
setIsVisible(false);
setModalVisible(false);
shouldAllowNavigation.current = true;
if (blockedNavigationAction.current) {
navigationRef.current?.dispatch(blockedNavigationAction.current);
Expand All @@ -43,7 +54,7 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, isEnabled = true}: Di
}
}}
onCancel={() => {
setIsVisible(false);
setModalVisible(false);
blockedNavigationAction.current = undefined;
}}
Comment on lines 56 to 59

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Call the cancel callback on native modal dismiss

The native discard-confirmation component now drives editable={!isDiscardModalVisible} in the merchant/description steps, which dismisses the keyboard when the modal opens, but its onCancel path never invokes the onCancel prop to re-focus the input. On iOS/Android, canceling the discard dialog therefore leaves the user on the same screen with the keyboard closed and no restored focus, so continuing to edit requires an extra tap (a regression from the previous flow where focus was preserved).

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZhenjaHorbach, this doesn't work in the latest main. I think it would be good to handle it in a follow-up pr. thanks.

Screen.Recording.1404-12-01.at.9.01.58.PM.mov

shouldHandleNavigationBack
Expand Down
26 changes: 17 additions & 9 deletions src/pages/iou/request/step/DiscardChangesConfirmation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {PlatformStackNavigationProp} from '@libs/Navigation/PlatformStackNa
import type {RootNavigatorParamList} from '@libs/Navigation/types';
import type DiscardChangesConfirmationProps from './types';

function DiscardChangesConfirmation({getHasUnsavedChanges, onCancel, isEnabled = true}: DiscardChangesConfirmationProps) {
function DiscardChangesConfirmation({getHasUnsavedChanges, onVisibilityChange, onCancel, isEnabled = true}: DiscardChangesConfirmationProps) {
const navigation = useNavigation<PlatformStackNavigationProp<RootNavigatorParamList>>();
const isFocused = useIsFocused();
const {translate} = useLocalize();
Expand All @@ -20,6 +20,14 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, onCancel, isEnabled =
const shouldNavigateBack = useRef(false);
const isConfirmed = useRef(false);

const setModalVisible = useCallback(
(nextVisible: boolean) => {
setIsVisible(nextVisible);
onVisibilityChange?.(nextVisible);
},
[onVisibilityChange],
);

useBeforeRemove(
useCallback(
(e) => {
Expand All @@ -29,9 +37,9 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, onCancel, isEnabled =

e.preventDefault();
blockedNavigationAction.current = e.data.action;
navigateAfterInteraction(() => setIsVisible((prev) => !prev));
navigateAfterInteraction(() => setModalVisible(true));
},
[getHasUnsavedChanges, isFocused, isEnabled],
[getHasUnsavedChanges, isFocused, isEnabled, setModalVisible],
),
isEnabled && isFocused,
);
Expand Down Expand Up @@ -59,20 +67,20 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, onCancel, isEnabled =
}
// Navigation.navigate() rerenders the current page and resets its states
window.history.go(1);
navigateAfterInteraction(() => setIsVisible((prev) => !prev));
navigateAfterInteraction(() => setModalVisible(true));
});

return unsubscribe;
}, [navigation, getHasUnsavedChanges, isFocused, isEnabled]);
}, [navigation, getHasUnsavedChanges, isFocused, isEnabled, setModalVisible]);

useEffect(() => {
if ((isFocused && isEnabled) || !isVisible) {
return;
}
setIsVisible(false);
setModalVisible(false);
blockedNavigationAction.current = undefined;
shouldNavigateBack.current = false;
}, [isFocused, isVisible, isEnabled]);
}, [isFocused, isVisible, isEnabled, setModalVisible]);

const navigateBack = useCallback(() => {
if (blockedNavigationAction.current) {
Expand All @@ -95,10 +103,10 @@ function DiscardChangesConfirmation({getHasUnsavedChanges, onCancel, isEnabled =
cancelText={translate('common.cancel')}
onConfirm={() => {
isConfirmed.current = true;
setIsVisible(false);
setModalVisible(false);
}}
onCancel={() => {
setIsVisible(false);
setModalVisible(false);
blockedNavigationAction.current = undefined;
shouldNavigateBack.current = false;
}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
type DiscardChangesConfirmationProps = {
/** Checks if there are any unsaved changes */
getHasUnsavedChanges: () => boolean;

/** Notifies when the discard modal visibility changes */
onVisibilityChange?: (isVisible: boolean) => void;

/** Called when the modal is closed without confirming */
onCancel?: () => void;

/** Whether the discard confirmation should be enabled */
isEnabled?: boolean;
};

Expand Down
3 changes: 3 additions & 0 deletions src/pages/iou/request/step/IOURequestStepDescription.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ function IOURequestStepDescription({

const [currentDescription, setCurrentDescription] = useState(currentDescriptionInMarkdown);
const [isSaved, setIsSaved] = useState(false);
const [isDiscardModalVisible, setIsDiscardModalVisible] = useState(false);
const shouldNavigateAfterSaveRef = useRef(false);
useRestartOnReceiptFailure(transaction, reportID, iouType, action);
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
Expand Down Expand Up @@ -211,6 +212,7 @@ function IOURequestStepDescription({
label={translate('moneyRequestConfirmationList.whatsItFor')}
accessibilityLabel={translate('moneyRequestConfirmationList.whatsItFor')}
role={CONST.ROLE.PRESENTATION}
editable={!isDiscardModalVisible}
autoGrowHeight
maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight}
shouldSubmitForm
Expand All @@ -222,6 +224,7 @@ function IOURequestStepDescription({
</View>
</FormProvider>
<DiscardChangesConfirmation
onVisibilityChange={setIsDiscardModalVisible}
onCancel={() => {
// eslint-disable-next-line @typescript-eslint/no-deprecated
InteractionManager.runAfterInteractions(() => {
Expand Down
3 changes: 3 additions & 0 deletions src/pages/iou/request/step/IOURequestStepMerchant.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function IOURequestStepMerchant({
const initialMerchant = isEmptyMerchant ? '' : merchant;
const [currentMerchant, setCurrentMerchant] = useState(initialMerchant);
const [isSaved, setIsSaved] = useState(false);
const [isDiscardModalVisible, setIsDiscardModalVisible] = useState(false);
const shouldNavigateAfterSaveRef = useRef(false);
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const currentUserAccountIDParam = currentUserPersonalDetails.accountID;
Expand Down Expand Up @@ -188,11 +189,13 @@ function IOURequestStepMerchant({
label={translate('common.merchant')}
accessibilityLabel={translate('common.merchant')}
role={CONST.ROLE.PRESENTATION}
editable={!isDiscardModalVisible}
ref={inputCallbackRef}
/>
</View>
</FormProvider>
<DiscardChangesConfirmation
onVisibilityChange={setIsDiscardModalVisible}
onCancel={() => {
// eslint-disable-next-line @typescript-eslint/no-deprecated
InteractionManager.runAfterInteractions(() => {
Expand Down
Loading