Details
I tested this diff stack by playing around with LoggedOutModal on my iOS simulator in my local environment:
- I navigated to LogInPanel and back
- I navigated to LegacyRegisterPanel and back
- I navigated to FullscreenSIWEPanel and back
- I navigated to QRCodeSignInNavigator and back
- I navigated to Registration and back
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
native/account/logged-out-modal.react.js | ||||||
---|---|---|---|---|---|---|
311–323 ↗ | (On Diff #40614) | These were just moved down (with useKeyboardHeight replaced by useRatchetingKeyboardHeight) | ||||
327–343 ↗ | (On Diff #40614) | These values are now determined in vanilla JavaScript by the getPanelPaddingTop worklet | ||||
350–353 ↗ | (On Diff #40614) | This block was just for initializing values. It arguably was never needed in the first place (we could have just initialized the values above) | ||||
354–364 ↗ | (On Diff #40614) | This logic is for handling when we set keyboardHeightValue to -1, which used to happen below in onPressLogIn and onPressRegister. The intention of that behavior was to wait for the keyboard to start appearing before starting the animation. The particular logic was there so that if the keyboard never appeared (eg. in an iOS simulator where the keyboard is disabled), then after the timeout clock ran out we would reset it to 0. In this diff, I got rid of the logic that sets keyboardHeightValue to -1. In my experience playing around with the animations it didn't seem very important. Perhaps it mattered more before, or perhaps the new Reanimated 2 logic is snappier for some reason. | ||||
365–375 ↗ | (On Diff #40614) | This logic was for deciding when to initiate a new animation. Reanimated 2 and 3 seem to handle this slightly differently. It looks like they allow you to set a shared value to a withTiming repeatedly, and then the C++ logic somehow intelligently decides whether a new animation needs to be initiated based on whether the target value has changed, and whether there is an animation in progress | ||||
376–379 ↗ | (On Diff #40614) | This was replaced by useRatchetingKeyboardHeight . I initially tried to use useAnimatedKeyboard from Reanimated directly, but found that the animation seems a bit slower than the keyboard. Here's a comparison:
| ||||
380–387 ↗ | (On Diff #40614) | This actually ran the animation, and is replaced by the withTiming call in the getPanelPaddingTop worklet | ||||
402–406 ↗ | (On Diff #40614) | This block was just for initializing values. It arguably was never needed in the first place (we could have just initialized the values above) | ||||
408–411 ↗ | (On Diff #40614) | This logic was for deciding when to initiate a new animation. Reanimated 2 and 3 seem to handle this slightly differently. It looks like they allow you to set a shared value to a withTiming repeatedly, and then the C++ logic somehow intelligently decides whether a new animation needs to be initiated based on whether the target value has changed, and whether there is an animation in progress | ||||
412–418 ↗ | (On Diff #40614) | This actually ran the animation, and is replaced by the withTiming call in the getPanelOpacity worklet | ||||
420–423 ↗ | (On Diff #40614) | This is replaced by the completion callback (third parameter) passed to withTiming in getPanelOpacity. I could have alternately used useAnimatedReaction instead of the completion callback – can change to that if preferred | ||||
native/flow-typed/npm/react-native-reanimated_v2.x.x.js | ||||||
548–554 ↗ | (On Diff #40614) | I added these types when I was initially trying to use useAnimatedKeyboard, but I decided against it (see video in inline comment above). Decided to keep the types for completeness's sake | ||||
native/keyboard/animated-keyboard.js | ||||||
76–90 ↗ | (On Diff #40614) | This replaces ratchetAlongWithKeyboardHeight |
native/account/logged-out-modal.react.js | ||
---|---|---|
465 ↗ | (On Diff #40614) | What is this replaced by? |
native/flow-typed/npm/react-native-reanimated_v2.x.x.js | ||
548 ↗ | (On Diff #40614) | In docs this is called AnimatedKeyboardInfo, can we call it that as well? |
550 ↗ | (On Diff #40614) | The docs say that state can have four values. Why do we have five? |
native/account/logged-out-modal.react.js | ||
---|---|---|
465 ↗ | (On Diff #40614) | It's actually not replaced. I'm not sure it needs to be, given that the Keyboard.dismiss() call will result in it eventually being set to 0... the animation is clean without it, and having this here breaks the clean separation introduced in D12207 |
native/flow-typed/npm/react-native-reanimated_v2.x.x.js | ||
548 ↗ | (On Diff #40614) | Sure, I'll rename it |
550 ↗ | (On Diff #40614) | The docs for version 3 have an additional section where they spell out the types explicitly, and an UNKNOWN is mentioned there that isn't covered in the "Possible values" section you reference. It's possible that UNKNOWN is only there for version 3, but I suspect it's there for version 2 as well, and I'm sure we gain anything from excluding it given we plan to upgrade to Reanimated 3 soon. Nevertheless, I can go ahead and check the actual source to confirm this |
native/flow-typed/npm/react-native-reanimated_v2.x.x.js | ||
---|---|---|
550 ↗ | (On Diff #40614) |