fix(iOS): modal becomes unresponsive with refresh control inside scrollable #48580
Conversation
facebook-github-bot
added
CLA Signed
kkafar
changed the title
facebook-github-bot
added
the
Shared with Meta
There was a problem hiding this comment.
@kkafar thanks for the PR. I left a couple of comments.
This behavior puzzles me. The didMoveToWindow function is from UIKit, so it should already and always be called from the main queue. Can you expand on when and how you encounter the problem?
| @@ -148,7 +148,9 @@ - (void)didMoveToWindow | |||
| { | |||
| [super didMoveToWindow]; | |||
| if (self.window) { | |||
| [self _attach]; | |||
| dispatch_async(dispatch_get_main_queue(), ^{ | |||
There was a problem hiding this comment.
Can you use RCTExecuteOnMainQueue instead? It avoids the jump if we are already on the main queue.
There was a problem hiding this comment.
It's all about placing the block at the end of the queue - see my response here: #48580 (comment)
| [self _attach]; | ||
| dispatch_async(dispatch_get_main_queue(), ^{ | ||
| [self _attach]; | ||
| }); | ||
| } else { | ||
| [self _detach]; |
There was a problem hiding this comment.
shouldn't we add the same in the detach? also, how come that didMoveToWindow is called in a queue that is not the main queue? These methods are called by UIKit, React Native is not calling them explicitly...
There was a problem hiding this comment.
I didn't notice any problems with detach behaviour therefore I've left the code intact.
I believe this is a timing issue rather. The method is executed on UI thread as expected. See below
Same here (puzzling part). I haven't had time yet to debug this thoroughly and as I described in the related issue #48579 I do not understand the error mechanism. However, from my testing I can say that the behaviour is 100% reliable (the bug happens always) and deferring execution of the code that sets the
and it is executed on main queue as expected. There is some timing issue however - some unknown yet operations, which are already scheduled on UI queue must be completed before the
I describe this in more detail in the issue report #48579 - the reproduction should be 100% reliable from my experience. |
|
Having said the above ☝️ I'm not sure yet this is the proper way to fix the problem - I've just confirmed that deferring setting the |
Summary:
Fixes #48579
Changelog:
[IOS][FIXED] - Modal becomes unresponsive with refresh control in scrollable
Test Plan:
https://snack.expo.dev/jWNIJRvKt6aScyidI-BGW starts to work!