[$2000] Dev: IOS - Chat - Console error on dismiss keyboard without sending message

[kbecciv]

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:

1. Go to any chat
2. Click on return button for multiples empty line
3. Dismiss the keyboard on click on outside

Expected Result:

Console error should not show

Actual Result:

Getting console error on dismiss the keyboard

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.27.1

Reproducible in staging?: yes

Reproducible in production?: yes

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

Screen.Recording.2023-06-06.at.7.18.39.PM.mov

Expensify/Expensify Issue URL:

Issue reported by: @niravkakadiya25

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686059495976739

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e4b6a6d4dee21067
• Upwork Job ID: 1668483496244523008
• Last Price Increase: 2023-09-05

[melvin-bot]

Triggered auto assignment to @tjferriss (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[melvin-bot]

Current assignee @tjferriss is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

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

[tjferriss]

I'm not able to reproduce as I don't have a mobile dev environment with access to the console. @AndrewGable or @sobitneupane are either of you able to reproduce the bug?

Also, if there is a way for me to reproduce the console error on staging or prod, please let me know :)

[sobitneupane]

are either of you able to reproduce the bug?

Yes, I can reproduce the issue.

Also, if there is a way for me to reproduce the console error on staging or prod, please let me know :)

No, I don't think it can be reproduced on staging or prod.

[sobitneupane]

@tienifr Can you please add more explanation to root cause analysis? Which animation we are talking about here? Why is onSelectionChange called many times even when we return/next line once?

[spcheema]

@sobitneupane Actually event is sent once but it occurs when a user press multiple return to simultaneouly.

[tienifr]

@sobitneupane sorry my previous solution was not a good one (doesn't work when the emoji suggestion list/keyboard is opened, still has the warning) so I posted a new one here and marked the above outdated.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Getting console error on dismiss the keyboard

What is the root cause of that problem?

This line will configure the layout animation

App/src/pages/home/report/ReportActionCompose.js

Line 276 in ebe28b8

LayoutAnimation.configureNext(LayoutAnimation.create(50, LayoutAnimation.Types.easeInEaseOut, LayoutAnimation.Properties.opacity));

.

When we open the keyboard/the emoji suggestion list shows, or any animation happening when we type in the Composer, that animation will conflict with the LayoutAnimation above and causes the warning.

This is a known issue of LayoutAnimation and has been mentioned many times like here and here.

What changes do you think we should make in order to solve the problem?

The LayoutAnimation.configureNext was added from this PR so that views like emoji suggestion list will appear smoothly with opacity effect. But the LayoutAnimation.configureNext is buggy as mentioned above so we should move to use react-native-reanimated for this instead. We should add a new component similar to OpacityView here

App/src/components/OpacityView.js

Line 38 in 93957f0

const OpacityView = (props) => {

that has opacity effect on mount/unmount and wrap the Views like emoji suggestion list with it.

Updated: So precisely the above has been done in this PR that was merged recently. So now all we need to do is to remove the LayoutAnimation.configureNext since it no longer serves any purpose (this was also mentioned earlier as an alternative solution in my initial proposal as well)

What alternative solutions did you explore? (Optional)

NA

[bernhardoj]

I think @allroundexperts is implementing reanimated into the suggestion component.

[sobitneupane]

@allroundexperts Can you please take a look and confirm if you are working on the transition to react-native-reanimated on ReportActionCompose.

[DJStartupAgile]

@allroundexperts
The warning message you’re seeing seems to be related to a layout animation in React Native. Layout animations can sometimes cause unexpected behavior when they conflict with certain native components, like the keyboard. In your case, it appears that a layout animation is being triggered at the same time as the keyboard is being dismissed. This could lead to an unstable state and hence the warning message. To avoid this, you might want to delay the start of the animation until after the keyboard has been fully dismissed. You can use the Keyboard module’s addListener method to listen for the 'keyboardDidHide' event and then start the animation. This way, you ensure the keyboard has fully dismissed before starting the animation, which should prevent the warning

[melvin-bot]

📣 @DJStartupAgile! 📣
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. 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.
2. 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.
3. 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>

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[DJStartupAgile]

Contributor details
Your Expensify account email: kayomelese4@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/kayowakmelese

[melvin-bot]

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

[melvin-bot]

@AndrewGable, @tjferriss, @sobitneupane Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@AndrewGable, @tjferriss, @sobitneupane Whoops! This issue is 2 days overdue. Let's get this updated quick!

[tjferriss]

Software Mansion is looking into the issue.

[tomekzaw]

Hey, Reanimated maintainer from Software Mansion here 👋

Thanks @ntdiary for your investigation (#20552 (comment)) as well as submitting the proposal. I think this should resolve the underlying issue. I believe we can even go a step further and remove the cleanup block completely:

    // Clean up
    // below line serves as this one uiManager->_layoutAnimationGroup = nil;, because we don't have access to the
    // private field
    [uiManager setNextLayoutAnimationGroup:nil];

Here's the full story. In #3332 we rewrote the mechanism of Layout Animations and used RCTLayoutAnimationGroup to control the removal of exiting views (see here).

However, in #3824 we got rid of [[RCTLayoutAnimation alloc] initWithDuration:0 config:@{}] in favor of another approach. Looks like the cleanup block is no longer necessary.

I will consult this change with the author of the aforementioned PR and most likely submit a PR that removes the problematic call.

[melvin-bot]

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

[AndrewGable]

Wonderful, thank you for the update @tomekzaw!

[melvin-bot]

@AndrewGable, @tjferriss, @sobitneupane Eep! 4 days overdue now. Issues have feelings too...

[tomekzaw]

Update: We came up with second fix, it looks like it works better but still sometimes the warning shows up. Working on it.

[melvin-bot]

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

[tjferriss]

Thanks @tomekzaw. When do you expect to have a PR up?

[melvin-bot]

@AndrewGable, @tjferriss, @sobitneupane Eep! 4 days overdue now. Issues have feelings too...

[tomekzaw]

@tjferriss I wanted to test out the PR in the Expensify App but at this point I can't even reproduce the original issue on the main branch (53ac4ee). Here's a video recording:

video.mov

[melvin-bot]

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

[tjferriss]

Agreed. I'm no longer able to reproduce the issue on staging.

[AndrewGable]

Let's reopen if we see it again!

[ntdiary]

Eh, this issue only occurs in the dev environment. Also, since this line of code below has been deleted, the steps in the OP can no longer be reproduced, but the steps in this comment can still be reproduced (the root case is still calling setNextLayoutAnimationGroup multiple times in the same batch). This is also why we did not adopt the previous PR.

App/src/pages/home/report/ReportActionCompose.js

Line 276 in ebe28b8

LayoutAnimation.configureNext(LayoutAnimation.create(50, LayoutAnimation.Types.easeInEaseOut, LayoutAnimation.Properties.opacity));

demo4.mp4

[tomekzaw]

Eh, this issue only occurs in the dev environment.

I guess it also happens in the release environment -- it's just that the effect is not visible because console.warn in release mode is just a no-op.

the steps in the OP can no longer be reproduced, but the steps in #20888 (review) can still be reproduced

Okay, I will check out the other issue you've linked and try to reproduce the problem.

edit: I've tried really hard but I still can't reproduce the issue on #20888 (review) on the current main. @ntdiary Can you try it as well?

[ntdiary]

I guess it also happens in the release environment -- it's just that the effect is not visible because console.warn in release mode is just a no-op.

@tomekzaw, yeah, your description is a bit more accurate. 😄

edit: I've tried really hard but I still can't reproduce the issue on #20888 (review) on the current main. @ntdiary Can you try it as well?

demo4.mp4

This video is what I tested on the latest main branch. Can you please share a video of your operations? We need to make sure emoji suggestions or mention suggestions appear first. 🙂
Also, have you modified the code like I did?
If so, we need to make sure that [uiManager setNextLayoutAnimationGroup:nil]; takes effect.

[tomekzaw]

@ntdiary Thanks for the response. For some reason, after I pulled the latest main, the default build variant was Release Production, that's why there was no warning 🤦 Sorry for the trouble, will look into this issue again.

edit: it looks like software-mansion/react-native-reanimated#4955 resolves the issue, we'll merge it and release in 3.5.0

edit: released in 3.5.0 🎉

[ntdiary]

Great, then we just need to wait for the upgrade here. : )

[tomekzaw]

Bumping to 3.5.0 in #27193