Fix bottom offset issue

[abdelmagied94]

I faced an issue with opened keyboard bottomOffest when an action sheet opened and then closed. When I deep into the code, I found that in this case onKeyboardWillShow is called two times (as shown in the first video). The issue happens because the two events fired in sequence withoutonKeyboardWillHide event in between so the calculations is failed. As I understood the safeAreaSupport should return the bottomOffest prop if provided, otherwise return the bottom safe are. Therefore, I adjust it to adapt this logic and this fixed the issue. Actually, I don't sure if my understanding is right so I don't know if there's another cases this will be fail or not 😅

The same issue happens when the chat is empty. I found the reason is that the keyboard events is registered in two places in this case (one in Messanger component and the other is FlatList). You can see this in the second video. I just wonder why we need to make these two subscriptions. The FlatList subscription is already fired even if it's empty.

Video 1
Video 2

P.S. We can also solve this by using debounce to neglect one of the two events. This can be used if the original function logic is needed to handle other cases.

[xcarpentier]

Hi @abdelmagied94 thanks :)

[xcarpentier]

Hi @hirbod!
Can you please look at it.
I think you have a role in it: dcf8f0a
Thanks

[xcarpentier]

Need to validate it with someone @hirbod ...

[hirbod]

This sounds reasonable. I’m going to grab that fix in my build and do some testing and will report back

[abdelmagied94]

I adjusted the bottomOffset default value to be null. Sorry, I forget to update it in the first commit.

[hirbod]

@abdelmagied94 @xcarpentier I checked the PR and its working fine on my end. Nothing is breaking and swipe-back gesture + actionsheet examples also work fine as before.

0.62.3 reverted the attempt to limit triggers on keyboardWillShow multiple times within 100ms, but that broke the recalculations especially in this action-sheet case.

So this PR is safe to merge on my end, but we might need to dig why multiple calls on keyboardwillshow are important (specially since keyboard handling works a bit different when you use react-native-screens)

See this and my comment + video. #1823

So debouncing might not bring the desired effect.

Edit: as far as my observations have revealed is that the second on keyboard call (dunno if this comes from message or flatlist) is the one we're look for, since the second trigger actually sets the actual height we're looking for

[abdelmagied94]

So this PR is safe to merge on my end, but we might need to dig why multiple calls on keyboardwillshow are important (specially since keyboard handling works a bit different when you use react-native-screens)

@hirbod I agree with you we need to find the root of the problem. This is a temporary fix.

See this and my comment + video. #1823

So debouncing might not bring the desired effect.

I think using debouncing is different from this fix #1853. In #1853, the second event is neglected and actually it is the correct one. However, if debouncing used, the first incorrect event will be neglected.

Edit: as far as my observations have revealed is that the second on keyboard call (dunno if this comes from message or flatlist) is the one we're look for, since the second trigger actually sets the actual height we're looking for

I find the first incorrect event is thrown from FlatList if the chat isn't empty and thrown from the two listeners if the chat is empty. You can observe this in the videos logs.

[abdelmagied94]

@xcarpentier @hirbod I just curious to know why we need two listeners when the chat is empty. I will be so grateful if you answer me. Thank you in advance.

[hirbod]

I can’t answer it. I’m just a small contributor and heavy user :). Might be an issue, might be some sort of historical quick and dirty fix.

Did you try to remove one of the events and observe any side effects? If not, we might want to remove one of the events and do some testing.

[abdelmagied94]

I removed the message one and I don't notice any side effects with the empty chat.

If not, we might want to remove one of the events and do some testing.

Let's see what is @xcarpentier opinion.

[xcarpentier]

Can you create a PR with your fix please? ;)

[abdelmagied94]

Can you create a PR with your fix please? ;)

@xcarpentier Done #1856