-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 2023-11-09] [HOLD #16935] [$500] Web - Clicking the back button brings back the workspace settings panel #28098
Comments
Triggered auto assignment to @dylanexpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~01ebbc655dc38b6b99 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to @joekaufmanexpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh ( |
Hi, Proposed Solutions: Back Button Handling: State Management: Conditional Rendering: Testing and Debugging: Regards |
📣 @husnaink93! 📣
|
Hi, Proposed Solutions: Back Button Handling: State Management: Conditional Rendering: Testing and Debugging: Regards Contributor details |
|
Hi, Proposed Solutions: Back Button Handling: State Management: Conditional Rendering: Testing and Debugging: Regards Contributor details |
|
Hi,
I have read your issue on Github and am going to share my proposed solution here.
Proposed Solutions:
Back Button Handling:
To mitigate the issue of the side panel appearing when users press the back button, it is imperative to manage the back button event within our application's source code. This necessitates the customization of the event handling mechanism. For Android, this customization is achievable by modifying the behavior encapsulated within the onBackPressed() method, while iOS would require the management of the UINavigationController.
State Management:
A comprehensive review of our application's state management is advisable. By monitoring the state of the side panel and implementing appropriate code, we can ensure that the panel is promptly closed when users engage the back button. Should our application utilize state management libraries such as Redux (in the case of React Native) or Provider (for Flutter), these resources can facilitate the implementation process.
Conditional Rendering:
The side panel's display can be adjusted through conditional rendering. This entails rendering the panel only under circumstances where its presence is warranted, ensuring that it remains hidden when users navigate via the back button or during specific user interactions.
Testing and Debugging:
Rigorous testing of our application is imperative, particularly in scenarios involving back button interactions. The utilization of debugging tools such as Android Studio or Xcode will aid in pinpointing the root cause of the issue and implementing effective solutions.
Regards
Husnain Khalid
Contributor details
Your Expensify account email: husnaink93@gmail.com
Upwork Profile Link: https://www.upwork.com/fl/husnainkhalid8
|
I suggest you to push Contributor details as new message, for bot to be able to parse it correctly :) |
|
ProposalPlease re-state the problem that we are trying to solve in this issue
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
What alternative solutions did you explore? (Optional) ResultScreen.Recording.2023-09-25.at.3.33.16.AM.mp4 |
I have a question: shouldn't the |
@dylanexpensify Going to unassign here as I was the duplicate assignee, and this doesn't need two BZ assignees. LMK if there's anything I can help with though! |
Improved ProposalSubject: Proposed Solutions for Back Button and Side Panel Issue Body: Dear Team, I have thoroughly reviewed the issue reported on Github regarding the unexpected appearance of the side panel when users press the back button. Here, I present a comprehensive approach to address this concern. 1. Back Button Event Handling: To prevent the side panel from appearing upon pressing the back button, we need to customize the back button event within our application's source code. For Android, we can modify the behavior within the 2. State Management: It is crucial to review and manage the state of our application, specifically the state of the side panel. By implementing code that closes the panel when the back button is pressed, we can ensure a smoother user experience. If our application leverages state management libraries like Redux (for React Native) or Provider (for Flutter), these tools can simplify the process. 3. Conditional Rendering: We can control the side panel's visibility using conditional rendering, rendering the panel only when necessary. This approach ensures that the panel remains hidden when users navigate using the back button or during specific user interactions. 4. Testing and Debugging: Thorough testing of the application is essential, especially in scenarios involving back button interactions. Debugging tools such as Android Studio or Xcode can help identify the root cause of the issue and facilitate the implementation of effective solutions. I hope these proposed solutions provide a clear path forward to address the issue. I look forward to your feedback. Contributor details |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
What alternative solutions did you explore? (Optional)
ResultScreencast.from.27-09-2023.02.32.03.webm |
reviewing |
Currently, after clicking on "New workspace", and then pressing the back button in browser, it not only navigates back to the workspaces list but also replaces the admin report by the concierge report. Screencast.from.05-10-2023.09.53.20.webmI think it is a bug. So what is the expected behavior? @danieldoglas please help check when you have a chance. Thanks |
I think it should go back to the workspace list, but not change the report that is currently being shown in the background @DylanDylann |
agree - have we factored this into the solution @DylanDylann? 🙇♂️ |
This issue will be hold based on #28573 (comment) @dylanexpensify |
still on hold |
hold |
@dylanexpensify this PR #28573 is just merged |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.94-2 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 2023-11-09. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
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:
|
BugZero Checklist:
Regression Test Proposal
|
payment upcoming! |
bump @dylanexpensify |
Payment summary:
Upwork offers sent! Please accept or request in NewDot! |
Payments sent! |
done! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
The side panel should not open when clicking back, instead app should load the previous chat page. This behavior is not repeatable unless it is a new account.
Actual Result:
App opens up the side panel when clicking back.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.73.0
Reproducible in staging?:
Reproducible in production?:
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
Notes/Photos/Videos: Any additional supporting documentation
2023-09-22.18.37.48.mp4
Recording.4743.mp4
Expensify/Expensify Issue URL:
Issue reported by: @Nathan-Mulugeta
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695397220000719
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: