Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App should navigate back to current chat report after closing status modal #31051

Merged
merged 5 commits into from
Nov 16, 2023
Merged

App should navigate back to current chat report after closing status modal #31051

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a25fe7c
app should navigate back to current chat report after closing status …
dukenv0307 Nov 8, 2023
faa6cde
fix lint issue
dukenv0307 Nov 8, 2023
d865dec
Merge branch 'main' into fix/30509-different-chat-report-opens-when-c…
dukenv0307 Nov 8, 2023
e036746
Merge branch 'main' into fix/30509-different-chat-report-opens-when-c…
dukenv0307 Nov 9, 2023
69368d3
update usecallback function
dukenv0307 Nov 10, 2023
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
11 changes: 10 additions & 1 deletion src/pages/settings/Profile/CustomStatus/StatusPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,16 @@ function StatusPage({draftStatus, currentUserPersonalDetails}) {
User.clearDraftCustomStatus();
};

const navigateBackToSettingsPage = useCallback(() => Navigation.goBack(ROUTES.SETTINGS_PROFILE, false, true), []);
const topMostReportID = Navigation.getTopmostReportId();

const navigateBackToSettingsPage = useCallback(() => {
if (topMostReportID) {
Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(topMostReportID));
} else {
Navigation.goBack(ROUTES.SETTINGS_PROFILE, false, true);
}
}, [topMostReportID]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we take topMostReportID inside the navigateBackToSettingsPage function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shubham1206agra I think we want the navigateBackToSettingsPage function to capture the latest value of topMostReportID which can change frequently, therefore it is better to leave const topMostReportID = Navigation.getTopmostReportId(); outside of the function and add topMostReportID as a dependency of useCallback hook

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I haven't used useCallback much recently, but if we put Navigation.getTopmostReportId(); inside navigateBackToSettingsPage, won't getTopmostReportId always get the most up to date topMostReportID
since getTopmostReportId will be called right when the value is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Beamanator On the second thought, you are right. I updated the code, please let me know if any concerns


const updateStatus = useCallback(() => {
User.updateCustomStatus({text: defaultText, emojiCode: defaultEmoji});

Expand Down
Loading