fix: allow Menu to reopen when hide animation callback never fires - #5051
fix: allow Menu to reopen when hide animation callback never fires#5051ErfanBagheri404 wants to merge 3 commits into
Conversation
Sorry, something went wrong.
On React Native >= 0.80 the hide animation completion callback is not delivered, so prevRendered.current stayed true and the menu could never be shown again after being dismissed. Reset prevRendered.current before starting the hide animation instead of in its completion callback. Fixes callstack#4763
| setRendered(false); | ||
| prevRendered.current = false; | ||
| focusFirstDOMNode(anchorRef.current); | ||
| }); |
There was a problem hiding this comment.
I suppose, moving prevRendered.current = false before hide animation can introduce a race. if the menu closes while its opening animation is running, the interrupted opening callback can set this back to true, preventing the menu from reopening
could we handle interrupted opening & hiding animations using finished and add a test for quickly closing & reopening the menu?
}).start(({ finished }) => {
if (!finished) {
return;
}
setMenuLayout({ width: 0, height: 0 });
setRendered(false);
prevRendered.current = false;
focusFirstDOMNode(anchorRef.current);
});
Sorry, something went wrong.
When the menu closes mid-opening animation, the interrupted callback was setting prevRendered.current = true, blocking the next open. Only set it when the animation finishes. Addresses review comment on callstack#5051.
MikitasK
left a comment
There was a problem hiding this comment.
Thanks @ErfanBagheri404 🙏
LGTM 👌
just one non-blocking suggestion: regression test covering close & reopen behavior would be helpful
Sorry, something went wrong.
Cover the scenario where the menu is dismissed then immediately reopened. Without the prevRendered.current reset in hide(), the interrupted opening animation callback could lock the menu closed.
Reviewers
1 more reviewerAt least 1 approving review is required to merge this pull request.
Fixes #4763
On React Native >= 0.80 the hide animation completion callback is not delivered, so prevRendered.current stayed true and the menu could never be shown again after being dismissed.
Reset prevRendered.current before starting the hide animation instead of in its completion callback.