-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
[HOLD for payment 2024-06-11] [$250] Tapping outside quickly after clear status leads the user going back to the report screen #39848
Comments
Triggered auto assignment to @jliexpensify ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Tapping outside quickly after clear status leads the user going back to the report screen What is the root cause of that problem?B/c we are navigating back here after interaction App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx Lines 109 to 110 in ec41a4b
when we quickly dismiss modal then it will run goBack on that route opening the report screen instead of the intended dismissing of the status modal What changes do you think we should make in order to solve the problem?we should use
Also apply same change here App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx Lines 94 to 95 in ec41a4b
What alternative solutions did you explore? (Optional)We can also remove the |
Interesting, I can repro but it doesn't take me to a Report - instead I get taken to my |
Job added to Upwork: https://www.upwork.com/jobs/~014d6d259b4d3f1b08 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
I also think this shoudl go in #vip-vsb as it deals with |
ProposalPlease re-state the problem that we are trying to solve in this issue.We are navigated to the last navigated route before profile if we dismiss model quickly after clearing status What is the root cause of that problem?When we call
Now if we have already dismissed the What changes do you think we should make in order to solve the problem?We can take help of the App/src/libs/Navigation/Navigation.ts Lines 354 to 355 in ec41a4b
This will help us to diff --git a/src/pages/settings/Profile/CustomStatus/StatusPage.tsx b/src/pages/settings/Profile/CustomStatus/StatusPage.tsx
index bb7be4a686..0912e610e7 100644
--- a/src/pages/settings/Profile/CustomStatus/StatusPage.tsx
+++ b/src/pages/settings/Profile/CustomStatus/StatusPage.tsx
@@ -107,7 +107,9 @@ function StatusPage({draftStatus, currentUserPersonalDetails}: StatusPageProps)
formRef.current?.resetForm({[INPUT_IDS.EMOJI_CODE]: ''});
InteractionManager.runAfterInteractions(() => {
- navigateBackToPreviousScreen();
+ if (Navigation.isDisplayedInModal()) {
+ navigateBackToPreviousScreen();
+ }
});
};
What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.The user goes back to report screen What is the root cause of that problem?When we save or clear status, we call App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx Lines 109 to 111 in 76c7fa9
What changes do you think we should make in order to solve the problem?We should cancel the interaction task if it's not called before the
What alternative solutions did you explore? (Optional)NA |
@jliexpensify I'm the reporter of this issue, I can take over this as C+. |
Hi @dukenv0307 - for fairness sake, I think it would be good to keep this with the auto-assigner. We currently don't have a documented process for what happens if a C+ reports a bug and wishes to be assigned - if you feel that the C+ who reports a bug should be the one assigned, you can present a Problem/Solution statement in #contributor-plus for review by our team and we can add it to our documentation, if we agree. Also bumping @thesahindia for reviews! |
Bumped @thesahindia on Slack, please review. If you're not able to take this on, I'll re-assign the issue. |
@GandalfGwaihir's proposal looks good to me! 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @GandalfGwaihir 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@thesahindia The proposal from @GandalfGwaihir will not correct in this case we clear the status and open quickly any modal by shortcut keyboard like cc @youssef-lr Screen.Recording.2024-04-12.at.22.40.52.mov |
Honestly, I'm not sure this is bug is worth fixing right now, how likely is this to be an issue for real world users? cc @jliexpensify |
@GandalfGwaihir let's hold off on working on this until we're sure we want to fix this. |
@youssef-lr Since the solution to fix all possible cases is very simple, I think we can fix all of this to not face other bugs like this in the further. |
@youssef-lr yeah sure, my branch is ready for PR, let me know once we decide to fix this |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.78-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-06-11. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
New summary, is this correct?
|
@jliexpensify I think there's a confusion here. I'm a reporter not C+. Should I get paid? Thanks |
Aha sorry - I was going off this Hmm, I don't think we pay out bug reporting anymore but let me double-check. Asking internally - https://expensify.slack.com/archives/C01SKUP7QR0/p1717558475261729 |
So we don't pay on bug reporting anymore @dukenv0307 - not sure why Melvin is including you. Apologies! |
It was a really minor issue which couldn't have been prevented. Also we don't need a test case here. |
Not overdue |
Ah sorry, I missed this one yesterday. Paying today! @thesahindia and @allgandalf - is there a checklist that needs to be done? |
Whoops sorry, ignore me - no checklist needed. |
Damn, job is expired. @allgandalf and @nkdengineer - can you accept the offers here? |
accepted @jliexpensify 👍 |
Cheers, just waiting on @nkdengineer |
Sorry @jliexpensify I don't have any Upwork Connects left so could not apply to the job. Would you mind sending the offer directly to my Upwork profile? TIA! |
@nkdengineer I sent you an offer yesterday, can you see / accept this? |
@jliexpensify Yes I accepted, thanks |
Everyone is paid, and the job is closed! @thesahindia and JMills, please refer to this summary for ND payments. |
$250 approved for @thesahindia |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.61-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712306439703269
Action Performed:
Expected Result:
The user should keep the side on the profile page
Actual Result:
The user goes back to report screen
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Recording.2953.mp4
Screen.Recording.2024-04-05.at.15.35.03.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @jliexpensifyThe text was updated successfully, but these errors were encountered: