App icon indicating copy to clipboard operation
App copied to clipboard

Room - No animation when returning to room settings after saving new room description

Open lanitochka17 opened this issue 1 year ago • 8 comments

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

  1. Go to https://staging.new.expensify.com/ and log in
  2. Navigate to the invoice room or any other room
  3. Click on the header
  4. Select the room description
  5. 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

View all open jobs on GitHub

lanitochka17 avatar Nov 26 '24 18:11 lanitochka17

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.

melvin-bot[bot] avatar Nov 26 '24 18:11 melvin-bot[bot]

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); or goBack() inside InteractionManager.runAfterInteractions. https://github.com/Expensify/App/blob/479edfc6a479e8bbbcdd446f1c96e5a3f2967bcf/src/pages/RoomDescriptionPage.tsx#L57-L63
  • We can do the same in RoomNamePage and other similar page if required.
  • We can also return early from submitForm if previousValue & newValue is 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

Krishna2323 avatar Nov 26 '24 18:11 Krishna2323

PROPOSAL UPDATED

  • Added alternative

Krishna2323 avatar Nov 26 '24 18:11 Krishna2323

Unable to repro... Animations are working fine for me(both chrome and safari). Is there any specific condition to repro?

ChavdaSachin avatar Nov 26 '24 18:11 ChavdaSachin

@johncschuster Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Dec 02 '24 09:12 melvin-bot[bot]

I'm catching up from being OOO for the holiday last week. Will get this slotted in tomorrow.

johncschuster avatar Dec 02 '24 22:12 johncschuster

@johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Dec 06 '24 09:12 melvin-bot[bot]

I couldn't action this today. I will check it out this weekend.

johncschuster avatar Dec 06 '24 23:12 johncschuster

Job added to Upwork: https://www.upwork.com/jobs/~021866175552123050816

melvin-bot[bot] avatar Dec 09 '24 17:12 melvin-bot[bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)

melvin-bot[bot] avatar Dec 09 '24 17:12 melvin-bot[bot]

@lanitochka17 is this reproducible 100% of the time, or are there certain conditions required to reproduce this reliably?

johncschuster avatar Dec 09 '24 17:12 johncschuster

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

  1. 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, goBack is called immediately after Report.updateDescription, not allowing time for any animations to complete.

  2. 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, the goBack function 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

  1. Update the submitForm function in src/pages/RoomDescriptionPage.tsx to use requestAnimationFrame for calling goBack.
  2. Test the change to ensure the animation issue is resolved and the navigation works as expected.

Code Changes

requestAnimationFrame(goBack);

Husejin avatar Dec 09 '24 21:12 Husejin

📣 @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:

  1. Make sure you've read and understood the contributing guidelines.
  2. 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.
  3. 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.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

melvin-bot[bot] avatar Dec 09 '24 21:12 melvin-bot[bot]

📣 @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:

  1. Make sure you've read and understood the contributing guidelines.
  2. 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.
  3. 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.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM 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]

Husejin avatar Dec 09 '24 21:12 Husejin

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

melvin-bot[bot] avatar Dec 09 '24 21:12 melvin-bot[bot]

@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!

melvin-bot[bot] avatar Dec 10 '24 09:12 melvin-bot[bot]

I am not able to reproduce this issue, @lanitochka17 could you plz recheck and confirm

Pujan92 avatar Dec 12 '24 12:12 Pujan92

@Pujan92, I can still reproduce this issue.

https://github.com/user-attachments/assets/699be3f3-9b11-43df-a210-6c6f44ec1b7e

Krishna2323 avatar Dec 13 '24 01:12 Krishna2323

@johncschuster, @Pujan92 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] avatar Dec 16 '24 09:12 melvin-bot[bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] avatar Dec 16 '24 16:12 melvin-bot[bot]

@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?

johncschuster avatar Dec 16 '24 23:12 johncschuster

did you do anything beyond what the reproduction steps said above?

@johncschuster, nope. Please make sure to edit the description before saving.

Krishna2323 avatar Dec 16 '24 23:12 Krishna2323

Thanks, @Krishna2323!

johncschuster avatar Dec 18 '24 00:12 johncschuster

@johncschuster, @Pujan92 Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Dec 18 '24 09:12 melvin-bot[bot]

@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

Pujan92 avatar Dec 19 '24 18:12 Pujan92

Triggered auto assignment to @jasperhuangg, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] avatar Dec 19 '24 18:12 melvin-bot[bot]

@johncschuster, @Pujan92, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Dec 23 '24 09:12 melvin-bot[bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] avatar Dec 23 '24 16:12 melvin-bot[bot]

@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!

melvin-bot[bot] avatar Dec 24 '24 09:12 melvin-bot[bot]

@johncschuster, @Pujan92, @jasperhuangg Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] avatar Dec 25 '24 09:12 melvin-bot[bot]