Skip to content

Commit

Permalink
dismiss modal on split bill
Browse files Browse the repository at this point in the history
  • Loading branch information
@luacmartins
luacmartins committed Oct 13, 2022
1 parent da9a895 commit 74dac7e
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/libs/actions/IOU.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ function splitBill(participants, currentUserLogin, amount, comment, currency, lo
transactionID: groupData.transactionID,
reportActionID: groupData.reportActionID,
}, onyxData);

Navigation.dismissModal();

This comment has been minimized.

Sign in to view

Copy link
@parasharrajat

parasharrajat Jul 19, 2023

Based on Expensify#17964 what is expected here?

  1. Modal should be closed completely and going back in browser history takes back to the Split Bill modal first page (amount page).
  2. Modal should be closed completely and going back does not open the Split modal page.

@bernhardoj

This comment has been minimized.

Sign in to view

Copy link
@bernhardoj

bernhardoj Jul 20, 2023

Owner

The 2nd one. However, this doesn't always work correctly.

Navigation.dismissModal(); can accept a targetReportID. If it's empty or equal to getTopmostReportId, it will simply pop all the RHP modals. In this case, going back won't open back the RHP.

https://github.com/Expensify/App/blob/391c41267ccb6dc14530ea552bf08772f99ce0ec/src/libs/Navigation/Navigation.js#L154-L162

But this is not the case when targetReportID exists and is not equal to getTopmostReportId. It will replace the RHP with the target report screen, and going back will open back the RHP. This issue is happening here and here which I believe you came from the 2nd issue.

This comment has been minimized.

Sign in to view

Copy link
@parasharrajat

parasharrajat Jul 20, 2023

Yes, I came for Expensify#22313. Thanks for confirming but I am not sure if that should be the correct behaviour. We are manipulating history again. We should all the users to go back to the split modal. Now, because of this, the logic is not fitting well.

This comment has been minimized.

Sign in to view

Copy link
@bernhardoj

bernhardoj Jul 20, 2023

Owner

I believe we want to have the same behavior with native, so the 2nd one should be the expected behavior here.

When we split bill, we dismiss the modal, so the RHP modal does not exist on the navigation stack anymore.
[LHN, Split bill] -> Split bill -> [LHN, Report]
So, pressing back should take the user back to the LHN. If I'm not wrong, this works as expected on native.

This comment has been minimized.

Sign in to view

Copy link
@parasharrajat

parasharrajat Jul 20, 2023

Oh, I see. Let me test this thoroughly.
}

/**
Expand Down

0 comments on commit 74dac7e

Please sign in to comment.