Add KeyboardAvoidingView #4339
Add KeyboardAvoidingView #4339
Conversation
|
This is the flicker we have right now. It returns
|
|
@terrysahaidak it seems this PR fixes the height issue, just linking it here for reference |
|
|
||
| <Modal | ||
| style={styles.sheetContainer} | ||
| visible={isModalVisible} |
There was a problem hiding this comment.
Hey @terrysahaidak, I've tested your example with the PR #4360.
If you look closely, there's still some flickering when opening the modal and it happens both with Reanimated's and React Native's KeyboardAvoidingView:
KeyboardAvoidingView from Reanimated
reanimated.mp4
KeyboardAvoidingView from React Native
react.native.mp4
I believe the root cause of this flicker is not the implementation of useAnimatedKeyboard hook but rather the fact that the visibility of <Modal> component is controlled using React state whereas the other part of the logic uses a shared value. In particular, there are two sources of truth and it takes 1-2 frames to render the changes from the JS thread when the React state is updated:
react-native-reanimated/app/src/examples/KeyboardAvoidingViewExample.tsx
Lines 109 to 110 in d019930
Instead, you should keep the state only in shared value and control the visibility of the modal via animated props on the UI thread:
import Animated, { useAnimatedProps } from 'react-native-reanimated';
const AnimatedModal = Animated.createAnimatedComponent(Modal);
const animatedProps = useAnimatedProps(() => {
return { visible: isModalVisible.value };
});
return <AnimatedModal {/* props */} animatedProps={animatedProps} />;Edit: looks like visible prop is available only on iOS, on Android it's implemented on JS side
## Summary This PR refactors the logic in REAKeyboardEventObserver to address some number of different issues: 1) no change notification when keyboard size changes due to change in language or showing suggestions bar 2) issue with keyboard height notification when opening modal (see #4339) 3) issue with non-zero height after keyboard hides completely. Here is the list of the main thing that this refactor changes: 1) We no longer use willShow/willHide/didShow/didHide notifications for keyboard and only use willChangeFrame which is the most reliable one and triggers also when the keyboard updates 2) We no longer use a technique where the keyboard size is determined based on the frame of the keyboard view. This was problematic as that frame would sometimes reflect a one-frame delayed size which resulted in the animation not coming down to 0 or not updating at all when immediately hidden or appear (by a modal). Instead, we use UIView animation to animate a complementary view that is placed outside of the screen visible area. We use UIView animation to animate height of that invisible view and then use display link to access presentation layer in order to read up-to-date height. There are few gatchas related to the new technique used: 1) Because we only look at frame change notification we need to properly determine the state of the keyboard. We do it based on the target height. If it is 0 then we consider the state to be either closing or closed – we select the final state based on whether there is animation running (closed state when no animation, or closing when we are animating). This prevents us from changing state to closing when keyboard is only becoming shorter (e.g. when text suggestion bar is closed). We handle opening/open state in similar way. 2) Due to the UIView animation technique we need to check presentationLayer.animationKeys to determine if the layer runs the animation. If it doesn't then we extract height from the view frame and not from the presentationLayer frame, as the presentationLayer may not have an up-to-date information (it will be delayed by one frame). ## Test plan Use example from #4339 and animated keyboard screen from Example app. Open/close text suggestion bar to see how the app reacts when keyboard is changing height and not just opening/closing.
|
Hey @terrysahaidak, since the PR with useAnimatedKeyboard improvements (#4360) just got merged, we'd like to proceed to your PR. Can you please merge main and fix linter errors so that we can review and test your changes? |
|
hey @tomekzaw! i'm gonna be working on this tomorrow, i'll let you know when it's ready. |
|
@terrysahaidak did you managed to work on this? Sorry for poking you, I'm eagerly waiting for this to come out since users are experiencing weird issues with |
…oard-avoiding-view
|
@efstathiosntonas yes, actually rebasing it right now and debugging stuff. |
| // we use `enabled === true` to be 100% compatible with original implementation | ||
| const bottomHeight = enabled === true ? bottom : 0; | ||
|
|
||
| console.log( |
There was a problem hiding this comment.
@terrysahaidak this console.log should be removed, right?
There was a problem hiding this comment.
Yes, these logs are there for debugging, they will be removed once it's actually ready for review, right now this PR is still in draft.
There was a problem hiding this comment.
@terrysahaidak just noticed the draft badge on the PR :face_palm:
| ); | ||
|
|
||
| const getBackwardCompatibleHeight = useWorkletCallback( | ||
| (keyboardHeight: number, frame: null | LayoutRectangle) => { |
There was a problem hiding this comment.
@terrysahaidak frame is shadowed from L54 const [frame, setFrame] = useState<LayoutRectangle | null>(null);
rename it to kFrame? 🤔
There was a problem hiding this comment.
@tomekzaw I believe that the Lint / check action should include shadow variables rule
|
@terrysahaidak |
…oard-avoiding-view
|
There is still a flicker after merging #4360 : #4351 (comment) |
|
Finally fixed the initial render bug, so the KeyboardAvoidingView is working fine on both iOS and Android, going to fix the example |
…oard-avoiding-view
## Summary This PR refactors the logic in REAKeyboardEventObserver to address some number of different issues: 1) no change notification when keyboard size changes due to change in language or showing suggestions bar 2) issue with keyboard height notification when opening modal (see software-mansion#4339) 3) issue with non-zero height after keyboard hides completely. Here is the list of the main thing that this refactor changes: 1) We no longer use willShow/willHide/didShow/didHide notifications for keyboard and only use willChangeFrame which is the most reliable one and triggers also when the keyboard updates 2) We no longer use a technique where the keyboard size is determined based on the frame of the keyboard view. This was problematic as that frame would sometimes reflect a one-frame delayed size which resulted in the animation not coming down to 0 or not updating at all when immediately hidden or appear (by a modal). Instead, we use UIView animation to animate a complementary view that is placed outside of the screen visible area. We use UIView animation to animate height of that invisible view and then use display link to access presentation layer in order to read up-to-date height. There are few gatchas related to the new technique used: 1) Because we only look at frame change notification we need to properly determine the state of the keyboard. We do it based on the target height. If it is 0 then we consider the state to be either closing or closed – we select the final state based on whether there is animation running (closed state when no animation, or closing when we are animating). This prevents us from changing state to closing when keyboard is only becoming shorter (e.g. when text suggestion bar is closed). We handle opening/open state in similar way. 2) Due to the UIView animation technique we need to check presentationLayer.animationKeys to determine if the layer runs the animation. If it doesn't then we extract height from the view frame and not from the presentationLayer frame, as the presentationLayer may not have an up-to-date information (it will be delayed by one frame). ## Test plan Use example from software-mansion#4339 and animated keyboard screen from Example app. Open/close text suggestion bar to see how the app reacts when keyboard is changing height and not just opening/closing.
|
There are still some bugs, for example the behaviour of animated and native KeyboardAvoidingView may differ a lot. Screen.Recording.2023-09-04.at.16.09.40.movThis is a very important change so expect several iterations of code reviews etc. and lot of testing. Therefore we are going to copy this code and fix it on our end. Thank you very much for your amazing work! #5064 |
|
@Latropos any update on this one? |
|
Hey @roryabraham, I've asked @szydlovsky to review the PR and compare it with existing implementations, will keep you updated |
Summary
This PR implements React Native KeyboardAvoidingView using
useAnimatedKeyboardhook.The goal of this implementation is to be always perfectly in sync with
useAnimatedKeyboardso we can animate things based on the keyboard state in the same frame.Another benefit of this implementation is that it doesn't rely on Layout Animation which sometimes makes things really hard.
Using Reanimated's KeyboardAvoidingView
2023-04-06.16.40.02.mov
Using React Native's KeyboardAvoidingView
2023-04-06.16.41.43.mov
Another benefit of this implementation is that we can actually introduce more properties and mechanics that are not as confusing as the current
behaviorandkeyboardVerticalOffsetprops no one knows how to properly pair.As part of this implementation, I'm adding an example I'm implementing right now for Expensify (Expensify/App#16356) but you can see it in other apps such as Slack and Messenger.
In chat applications, KeyboardAvoidingView usually pushes the scroll view to the top alongside input. But if you want to show a modal instead of the keyboard, for example, simulating InputAccessoryView, the keyboard hides without animation, and on the next frame the modal is rendered. So KeyboardAvoidingView no longer pushes the content since the keyboard is gone by now and all the content is now covered by our modal.
What we can do in this particular case is to
translateYthe content of the scroll view by the last keyboard height when the modal is visible.2023-04-06.15.31.41.mov
There is a bug left with
useAnimatedKeyboardwhich results into a few weird frames in this example, since it reports at some point:Test plan
Run KeyboardAvoidingView example.