Room - No animation when returning to room settings after saving new room description
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: 9.0.67.0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team
Action Performed:
precondition: user is an admin of workspace, enabled Invoice in workspace setting and sent at least one invoice
- Go to https://staging.new.expensify.com/ and log in
- Navigate to the invoice room or any other room
- Click on the header
- Select the room description
- Enter new description and click on Save
Expected Result:
There is an animation when returning to the room settings
Actual Result:
There is no animation when returning to the room settings
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/511bd655-1e75-4a25-a429-0ef7fbdfa477
Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Edited by proposal-police: This proposal was edited at 2024-11-26 18:33:46 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Room - No animation when returning to room settings after saving new room description
What is the root cause of that problem?
- Onyx update and navigation is called at the same time, due to that the animation is broken.
What changes do you think we should make in order to solve the problem?
- Call
Report.updateDescription(report.reportID, previousValue, newValue);orgoBack()insideInteractionManager.runAfterInteractions. https://github.com/Expensify/App/blob/479edfc6a479e8bbbcdd446f1c96e5a3f2967bcf/src/pages/RoomDescriptionPage.tsx#L57-L63 - We can do the same in
RoomNamePageand other similar page if required. - We can also return early from
submitFormifpreviousValue&newValueis the same.
What alternative solutions did you explore? (Optional)
- We can use
setNavigationActionToMicrotaskQueue.Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.navigate(backTo ?? ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID)));
Result
Unable to repro... Animations are working fine for me(both chrome and safari). Is there any specific condition to repro?
@johncschuster Eep! 4 days overdue now. Issues have feelings too...
I'm catching up from being OOO for the holiday last week. Will get this slotted in tomorrow.
@johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
I couldn't action this today. I will check it out this weekend.
Job added to Upwork: https://www.upwork.com/jobs/~021866175552123050816
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)
@lanitochka17 is this reproducible 100% of the time, or are there certain conditions required to reproduce this reliably?
Proposal: Fix Animation Issue on Room Description Menu Disappearance
Issue
When the "save" button is pressed on the Room Description page, the room description menu sometimes vanishes without an animation. This abrupt disappearance negatively impacts the user experience.
Root Cause
The issue occurs because the goBack function is called immediately after updating the description, not allowing the UI enough time to animate the disappearance of the menu.
Proposed Solution
To ensure the animation is properly handled, we can use the requestAnimationFrame function. This will schedule the goBack function to run after the next repaint, ensuring that the UI updates (including animations) are completed before navigating back.
Detailed Explanation
-
Current Implementation:
const submitForm = useCallback(() => { const previousValue = report?.description ?? ''; const newValue = description.trim(); Report.updateDescription(report.reportID, previousValue, newValue); goBack(); }, [report.reportID, report.description, description, goBack]);In this code,
goBackis called immediately afterReport.updateDescription, not allowing time for any animations to complete. -
Proposed Change:
const submitForm = useCallback(() => { const previousValue = report?.description ?? ''; const newValue = description.trim(); Report.updateDescription(report.reportID, previousValue, newValue); requestAnimationFrame(goBack); }, [report.reportID, report.description, description, goBack]);By using
requestAnimationFrame, thegoBackfunction is deferred until the next repaint. This ensures that the UI has time to process any pending updates, including animations, before navigating back.
Benefits
- Improved User Experience: Ensures smooth transitions and animations, providing a better user experience.
- Efficiency: Avoids the need for arbitrary delays, making the code more efficient and responsive.
Action Items
- Update the
submitFormfunction insrc/pages/RoomDescriptionPage.tsxto userequestAnimationFramefor callinggoBack. - Test the change to ensure the animation issue is resolved and the navigation works as expected.
Code Changes
requestAnimationFrame(goBack);
📣 @Husejin! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
📣 @Husejin! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details Your Expensify account email: <REPLACE EMAIL HERE> Upwork Profile Link: <REPLACE LINK HERE>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: [https://www.upwork.com/freelancers/~01155e39e78b57de1f]
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
@johncschuster @Pujan92 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
I am not able to reproduce this issue, @lanitochka17 could you plz recheck and confirm
@Pujan92, I can still reproduce this issue.
https://github.com/user-attachments/assets/699be3f3-9b11-43df-a210-6c6f44ec1b7e
@johncschuster, @Pujan92 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@Krishna2323 did you do anything beyond what the reproduction steps said above? If so, can you clarify the steps you took to reproduce the issue?
did you do anything beyond what the reproduction steps said above?
@johncschuster, nope. Please make sure to edit the description before saving.
Thanks, @Krishna2323!
@johncschuster, @Pujan92 Eep! 4 days overdue now. Issues have feelings too...
@Krishna2323's alternate solution of using setNavigationActionToMicrotaskQueue seems to be a common pattern to let execute the Onyx update first and then navigation will solve the issue here. We can proceed with @Krishna2323's proposal.
Let's try to find other places with similar implementation and apply the fix there too(for eg. NotificationPreferencePage, WriteCapabilityPage, ...)
Thanks @Husejin for the proposal, plz go through this doc and use the appropriate proposal template for your next proposals.
🎀👀🎀 C+ reviewed
Triggered auto assignment to @jasperhuangg, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@johncschuster, @Pujan92, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@johncschuster @Pujan92 @jasperhuangg this issue is now 4 weeks old, please consider:
- Finding a contributor to fix the bug
- Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
- If you have any questions, don't hesitate to start a discussion in #expensify-open-source
Thanks!
@johncschuster, @Pujan92, @jasperhuangg Eep! 4 days overdue now. Issues have feelings too...