Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix useAnimatedKeyboard not giving proper value when using modal #5916

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

maciekstosio
Copy link
Contributor

@maciekstosio maciekstosio commented Apr 19, 2024

Summary

Fixes #5754

Because modals are open next to the root view in view hierarchy, they are not observed by InsetManager. In this PR I added separate class that manages opened dialogs, so we can listen to changes on modals too. The assumption is that the useAnimatedKeyboard gives the value of any opened keyboard, not only the one in current view.

image
Before After
before.useAnimatedKeyboardModal.mov
after.useAnimatedKeyboard.modal.mov

Test plan

Test component
import * as React from 'react';
import {
  TextInput,
  View,
  Button,
  Modal,
  SafeAreaView,
  StyleSheet,
} from 'react-native';
import Animated, {
  useAnimatedKeyboard,
  useAnimatedReaction,
  useAnimatedStyle,
} from 'react-native-reanimated';

function MyModal({ visible, hide }) {
  return (
    <Modal transparent visible={visible} onDismiss={hide} onRequestClose={hide} onLayout={(layout) => console.log('layout', layout)}>
      <View style={styles.modalContainer}>
        <View style={styles.modalContent}>
          <Button onPress={hide} title="Close Modal" />
          <TextInput style={styles.textInput} placeholder="Inside a modal" />
          <KeyboardHeightResponder />
        </View>
      </View>
    </Modal>
  );
}

function KeyboardHeightResponder() {
  const keyboard = useAnimatedKeyboard();
  const style = useAnimatedStyle(() => ({
    width: 50,
    height: 50,
    borderRadius: 25,
    backgroundColor: 'red',
    transform: [{ translateY: -keyboard.height.value }],
  }));

  useAnimatedReaction(
    () => keyboard.height,
    (height) => console.log(height.value)
  );

  return <Animated.View style={style} />;
}

export default function EmptyExample() {
  const [visible, setVisible] = React.useState(false);

  return (
    <>
      <SafeAreaView style={styles.container}>
        <Button onPress={() => setVisible(true)} title="Open Modal" />
        <TextInput style={styles.textInput} placeholder="Outside a modal" />
        <KeyboardHeightResponder />
      </SafeAreaView>
      <MyModal visible={visible} hide={() => setVisible(false)} />
    </>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'space-between',
    backgroundColor: '#ecf0f1',
    paddingHorizontal: 8,
    paddingBottom: 50,
    paddingTop: 8,
  },
  modalContainer: {
    ...StyleSheet.absoluteFill,
    flex: 1,
    justifyContent: 'center',
    backgroundColor: 'rgba(0,0,0,0.2)',
  },
  modalContent: {
    justifyContent: 'space-between',
    backgroundColor: 'white',
    borderRadius: 24,
    padding: 24,
    minHeight: 256,
  },
  textInput: {
    paddingHorizontal: 8,
    paddingVertical: 4,
    borderRadius: 8,
    backgroundColor: 'silver',
    fontSize: 18,
    textAlign: 'center',
  },
});

Base automatically changed from @maciekstosio/Investigate-unexpected-space-when-using-useAnimatedKeyboard to main May 1, 2024 08:11
@kirillzyusko
Copy link
Contributor

Hey @maciekstosio

Have you tested Fabric architecture? I've applied changes from this branch and it crashes app immediately when modal appears with:

FATAL EXCEPTION: main

com.facebook.react.uimanager.IllegalViewOperationException: Trying to resolve view with tag 480 which doesn't exist

FATAL EXCEPTION: main
Process: com.keyboardcontrollerfabricexample, PID: 3726
com.facebook.react.uimanager.IllegalViewOperationException: Trying to resolve view with tag 480 which doesn't exist
	at com.facebook.react.uimanager.NativeViewHierarchyManager.resolveView(NativeViewHierarchyManager.java:102)
	at com.facebook.react.uimanager.UIManagerModule.resolveView(UIManagerModule.java:870)
	at com.swmansion.reanimated.NodesManager.handleEvent(NodesManager.java:337)
	at com.swmansion.reanimated.NodesManager.onEventDispatch(NodesManager.java:320)
	at com.facebook.react.uimanager.events.FabricEventDispatcher.dispatchEvent(FabricEventDispatcher.java:43)

@maciekstosio
Copy link
Contributor Author

Hi @kirillzyusko! I did test it on Fabric and it worked on example form the PR. I added try catch when resolving the view, that may help with your problem 42c1ad3. If it won't, please provide a reproducible repo, I'll take a look.

@kirillzyusko
Copy link
Contributor

@maciekstosio hey, yes, I'll provide a repo 👍

@kirillzyusko
Copy link
Contributor

@maciekstosio it seems like I tested your PR against RN 0.73.4 (though now if I try to consume it, I'm getting an error that REA from branch is incompatible with RN 0.73.4).

Did you test your PR on RN 0.73 or only RN 0.74? 🤔

@maciekstosio
Copy link
Contributor Author

@kirillzyusko I didn't test it on Fabric on RN 0.73, as we support Fabric since 0.74

@kirillzyusko
Copy link
Contributor

@maciekstosio I have tried to test RN 0.74 + Fabric but for me it looked like modals were kind of broken there (I could open a modal, but window callbacks were not fired).

But if you tested it and it worked good in your case, then I think it's okay - I will try to test everything again more carefully next week.

Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to solve some problem, but it's great to hear that it's working 🎉

Comment on lines 104 to 106

private ModalActivityManager mModalActivityManager;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private ModalActivityManager mModalActivityManager;
private ModalActivityManager mModalActivityManager;

@@ -340,6 +346,17 @@ public void onEventDispatch(Event event) {
}

private void handleEvent(Event event) {
String eventName = event.getEventName();

if (eventName.equals(ModalActivityManager.OPEN_MODAL_EVENT_NAME)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remind me why we weren't able to add a listener to a screen to receive a notification about opening a modal?

Copy link
Contributor Author

@maciekstosio maciekstosio Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check if I could use something else.


private final HashMap<Integer, WindowsInsetsManager> insetsManagersMap = new HashMap<>();

public ModalActivityManager(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have more than one instance of ModalActivityManager?

@maciekstosio maciekstosio force-pushed the @maciekstosio/Fix-useAnimatedKeyboard-not-giving-proper-value-when-uisng-modal branch from fdba215 to 4ae9b77 Compare June 14, 2024 15:11
…ving-proper-value-when-uisng-modal

pick bf61d254 feat: extract shared example and use in in both Example and FabricExample
@maciekstosio
Copy link
Contributor Author

maciekstosio commented Jun 20, 2024

I tested few more approaches to tackle detecting Dialog open, but none of them seems like good fit:


  • Add addEventListener on NodesManager - that would make registering modal separate from other logic, but the main problem with listening to all events remains.
  • addWindowFocusChangeListener with onWindowFocusChange and WindowInspector.getGlobalWindowViews() - that allows use do detect new dialogs, but we don’t have access to Window, which we need to set setDecorFitsSystemWindows.



Other hacky solutions on hold:

  • Try to push flags on DecorView (which is something that probably setDecorFirstSystemWindows does under the hood) - the flags are deprecated and probably not all of them can be set easily.
  • Contribute to RN with a change that modals can trigger setDecorFitsSystemWindows using some proper - thanks to that I wouldn’t need Window and could just attach listeners on decor view, but still need to guess using onWindowFocusChange and WindowInspector when the modal is opened.


Preferable solution in research:


  • Add possibility to register listener on RN modals. 




For now we’re leaving it on hold and considering RN contribution that would make the solution less hacky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useAnimatedKeyboard does not work for Android when the focused TextInput is inside a Modal
3 participants