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

Fails to avoid keyboard if TextInput is focused while keyboard is closing #516

Closed
thisisgit opened this issue Jul 22, 2024 · 14 comments
Closed
Assignees
Labels
KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component question You wanted to clarify something about the usage of the library or have a question about something

Comments

@thisisgit
Copy link

Describe the bug
Hi, in a list of TextInputs, what I'm trying to achieve here is to create new item and focus it when return key is pressed from the keyboard.
This results in closing initially opened keyboard since blur is triggered when return key is pressed, and reopen the keyboard after new item is added and focused.

Newly added TextInput seems to avoid the keyboard just fine if the time between closing the keyboard and focusing new TextInput is very short, but once there's a delay in between those two actions, it doesn't avoid the keyboard.
The delay can be caused due to:

  1. Rendering added item takes time
  2. Adding new item and updating list is done by an API request

To simulate and reproduce this bug, I gave setTimeout of 100ms before adding new item. You can check it on addAndFocus function of below code.
I'm also posting a recorded video of this so I believe it'll be a lot easier to understand the issue by checking the video first.

Code snippet

import React, { useCallback, useEffect, useRef, useState } from 'react';
import { FlatList, ListRenderItemInfo, StyleSheet } from 'react-native';
import { TextInput } from 'react-native-gesture-handler';
import { KeyboardAwareScrollView } from 'react-native-keyboard-controller';

type ListItem = {
  id: string;
  text: string;
};

const DATA: ListItem[] = [
  {
    id: '1',
    text: 'Test1',
  },
  {
    id: '2',
    text: 'Test2',
  },
  {
    id: '3',
    text: 'Test3',
  },
  {
    id: '4',
    text: 'Test4',
  },
  {
    id: '5',
    text: 'Test5',
  },
  {
    id: '6',
    text: 'Test6',
  },
  {
    id: '7',
    text: 'Test7',
  },
];

type Props = ListItem & {
  isFocused: boolean;
  onSubmit: () => void;
};
const Item = ({ id, text, isFocused, onSubmit }: Props) => {
  const inputRef = useRef<TextInput | null>(null);

  useEffect(() => {
    if (isFocused) {
      inputRef.current?.focus();
    }
  }, []);

  return (
    <TextInput
      ref={inputRef}
      style={styles.input}
      value={text}
      onSubmitEditing={onSubmit}
    />
  );
};

export const TestKeyboardAwareList = () => {
  const [data, setData] = useState<ListItem[]>(DATA);
  const addedItemIdRef = useRef<string | undefined>();

  const addAndFocus = useCallback(() => {
    setTimeout(() => {
      const newItemId = new Date().getTime().toString();
      addedItemIdRef.current = newItemId;
      setData((prev) => [...prev, { id: newItemId, text: '' }]);
    }, 100);
  }, []);

  const renderItem = useCallback(
    ({ item }: ListRenderItemInfo<ListItem>) => {
      const isFocused = addedItemIdRef.current === item.id;
      return <Item {...item} isFocused={isFocused} onSubmit={addAndFocus} />;
    },
    [addAndFocus]
  );

  return (
    <FlatList
      data={data}
      renderItem={renderItem}
      keyExtractor={(item) => item.id}
      keyboardDismissMode={'interactive'}
      renderScrollComponent={(props) => {
        return <KeyboardAwareScrollView {...props} />;
      }}
    />
  );
};

const styles = StyleSheet.create({
  input: {
    height: 80,
    borderWidth: 1,
    width: '100%',
  },
});

Repo for reproducing
Import above component in any screen

To Reproduce
Steps to reproduce the behavior:

  1. Tap on last item to focus it
  2. Tap return button to add new item at the bottom of the list
  3. New item is added and focused but it won't avoid the keyboard

Expected behavior
Added item should avoid the keyboard

Screenshots

Simulator.Screen.Recording.-.iPhone.15.-.2024-07-22.at.17.52.29.mp4

Smartphone (please complete the following information):

  • Desktop OS: MacOS 14.5
  • Device: iPhone 15 simulator
  • OS: iOS 17.4
  • RN version: 0.74.2
  • RN architecture: old
  • JS engine: Hermes
  • Library version: 1.12.5

Additional context
n/a

@kirillzyusko
Copy link
Owner

kirillzyusko commented Jul 22, 2024

@thisisgit have you tried to add blurOnSubmit={false}? In this case the keyboard will not be closed automatically when you press enter (and you can control yourself whether the keyboard needs to be closed or you need to move the focus to the next input). I believe this prop will give you a better control of the keyboard management and you will not have race conditions 🤔 Am I right? 👀

If the problem even with blurOnSubmit={false} remains, then let me know - I'll try to reproduce it on my end 👍

@kirillzyusko kirillzyusko added the question You wanted to clarify something about the usage of the library or have a question about something label Jul 22, 2024
@thisisgit
Copy link
Author

@kirillzyusko You're right, blurOnSubmit={false} works since it won't close the keyboard when return key is pressed.

However it's a different story if we would want to support multiline and trigger submit when return key is pressed.
When multiline property is set to true, return key now adds a newline character instead of triggering submit. AFAIK there's no workaround to prevent newline character from being added when multiline is set to true. Here're related issues:

  1. No way to prevent newline character in multiline TextInput (without flicker) facebook/react-native#28535
  2. https://stackoverflow.com/questions/38068675/how-to-prevent-new-line-when-enter-is-pressed-in-react-native/49152780#49152780

Therefore TextInput should have blurOnSubmit={true} and multiline={true} to support above requirements.

I would also prefer to not close the keyboard when return key is pressed since that looks cleaner. But too bad they don't support it yet with multiline={true}. However this comment says they may be able to support it once Fabric is up (and hey, it's already 2024 and is now available🙄)

Now I see that I should have added this in the Additional Context. Sorry for that.

@kirillzyusko
Copy link
Owner

@thisisgit you are right with multiline={true} you need to have blurOnSubmit={true}, but how do you close the keyboard with multiline={true}? I think in this case you should call Keyboard.dismiss()?

In your example you were using single line inputs, so I thought that blurOnSubmit={false} may help you 🤷‍♂️

Do I correctly understand that you are using multiline={true} and blurOnSubmit={true} and when you press return then keyboard gets closed?

@thisisgit
Copy link
Author

@kirillzyusko Yup, to summarize:

  1. multiline={true} & blurOnSubmit={false}: hitting return key adds newline without closing the keyboard
Simulator.Screen.Recording.-.iPhone.15.-.2024-07-22.at.19.37.42.mp4
  1. multiline={true} & blurOnSubmit={true}: hitting return key triggers onSubmitEditing and closes the keyboard
Simulator.Screen.Recording.-.iPhone.15.-.2024-07-22.at.19.39.31.mp4
And here's the updated code of `multiline={true}` & `blurOnSubmit={true}`
import React, { useCallback, useEffect, useRef, useState } from 'react';
import { FlatList, ListRenderItemInfo, StyleSheet } from 'react-native';
import { TextInput } from 'react-native-gesture-handler';
import { KeyboardAwareScrollView } from 'react-native-keyboard-controller';

type ListItem = {
  id: string;
  text: string;
};

const DATA: ListItem[] = [
  {
    id: '1',
    text: 'Test1',
  },
  {
    id: '2',
    text: 'Test2',
  },
  {
    id: '3',
    text: 'Test3',
  },
  {
    id: '4',
    text: 'Test4',
  },
  {
    id: '5',
    text: 'Test5',
  },
  {
    id: '6',
    text: 'Test6',
  },
  {
    id: '7',
    text: 'Test7',
  },
];

type Props = ListItem & {
  isFocused: boolean;
  onSubmit: () => void;
};
const Item = ({ id, text, isFocused, onSubmit }: Props) => {
  const inputRef = useRef<TextInput | null>(null);

  useEffect(() => {
    if (isFocused) {
      inputRef.current?.focus();
    }
  }, []);

  return (
    <TextInput
      ref={inputRef}
      style={styles.input}
      defaultValue={text}
      multiline={true}
      blurOnSubmit={true}
      onSubmitEditing={onSubmit}
    />
  );
};

export const TestKeyboardAwareList = () => {
  const [data, setData] = useState<ListItem[]>(DATA);
  const addedItemIdRef = useRef<string | undefined>();

  const addAndFocus = useCallback(() => {
    // Using setTimeout to simulate adding new item via API request
    setTimeout(() => {
      const newItemId = new Date().getTime().toString();
      addedItemIdRef.current = newItemId;
      setData((prev) => [...prev, { id: newItemId, text: '' }]);
    }, 100);
  }, []);

  const renderItem = useCallback(
    ({ item }: ListRenderItemInfo<ListItem>) => {
      const isFocused = addedItemIdRef.current === item.id;
      return <Item {...item} isFocused={isFocused} onSubmit={addAndFocus} />;
    },
    [addAndFocus]
  );

  return (
    <FlatList
      data={data}
      renderItem={renderItem}
      keyExtractor={(item) => item.id}
      keyboardDismissMode={'interactive'}
      renderScrollComponent={(props) => {
        return <KeyboardAwareScrollView {...props} />;
      }}
    />
  );
};

const styles = StyleSheet.create({
  input: {
    height: 80,
    borderWidth: 1,
    width: '100%',
    fontSize: 24,
  },
});

@kirillzyusko
Copy link
Owner

@thisisgit thank you for such detailed explanation! I debugged your code. The problem comes to the fact that view is not laid out on a native side when you set a focus:

{
  "eventName": "onFocusedInputLayoutChanged",
  "layout": {
    "absoluteX": 0, "absoluteY": 59.666666666666664, "height": 0, "width": 0, "x": 0, "y": 0
  },
  "parentScrollViewTarget": 3327,
  "target": 3379
}

I slightly modified your code:

    setTimeout(() => {
      if (isFocused) {
        inputRef.current?.focus();
      }
    }, 16);

And now it works properly. Can you check on your side whether such code produces acceptable result for you?

I'm thinking on how to handle such cases on a native side, but don't have any ideas at the moment because current codebase heavily relies on the fact that we can measure TextInput before keyboard events. But with your code we break that rule, because view is not laid out yet and we actually set focus to that input (so we already have an upcoming keyboard event, but measurement will produce incorrect output).

Anyway - curious to hear whether such workaround works for you (i. e. to add 16ms on JS before .focus() to be sure that view is laid out). In my understanding that would be the ideal solution, because we still rely on event order that library relies on and you just add a small code piece that handles layout management and assures that view can be measure before setting a focus).

@kirillzyusko kirillzyusko added the KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component label Jul 22, 2024
@thisisgit
Copy link
Author

@kirillzyusko I tried adding 16ms timeout on both above example and on my app and can confirm that it's working good 👍

I wasn't suspecting view not laid on the native side at all since focus event is being triggered in useEffect.
And most of all, removing setTimeout from addAndFocus function just works:

const addAndFocus = useCallback(() => {
    // setTimeout(() => {
    //   const newItemId = new Date().getTime().toString();
    //   addedItemIdRef.current = newItemId;
    //   setData((prev) => [...prev, { id: newItemId, text: '' }]);
    // }, 100);

    // Removing setTimeout here avoids the keyboard
    const newItemId = new Date().getTime().toString();
    addedItemIdRef.current = newItemId;
    setData((prev) => [...prev, { id: newItemId, text: '' }]);
  }, []);
Simulator.Screen.Recording.-.iPhone.15.-.2024-07-22.at.23.58.54.mp4

Or giving setTimeout long enough to wait for keyboard to completely dismiss also works:

const addAndFocus = useCallback(() => {
    setTimeout(() => {
      const newItemId = new Date().getTime().toString();
      addedItemIdRef.current = newItemId;
      setData((prev) => [...prev, { id: newItemId, text: '' }]);
      // Or give longer timeout to wait for keyboard to dismiss completely
    }, 500);
  }, []);
Simulator.Screen.Recording.-.iPhone.15.-.2024-07-23.at.00.02.12.mp4

Note that I didn't apply the 16ms timeout fix in above examples.

That's why I was suspecting maybe the issue is coming from when trying to measure TextInput while keyboard is animating. Because content height of FlatList shrinks/expands when TextInput is focused/blurred and may fetch the measurement of TextInput which layout change is in progress.

Anyways, thanks to you, I'll stick to the 16ms solution. I also found out calling inputRef.current?.focus() on onLayout of TextInput also works but I'm afraid of side effects since onLayout is also triggered when layout of the item changes.

But I'm curious, isn't view supposed to be already laid out in the native side when useEffect is called in the component?

@kirillzyusko
Copy link
Owner

I also found out calling inputRef.current?.focus() on onLayout of TextInput also works but I'm afraid of side effects since onLayout is also triggered when layout of the item changes.

I think you can create an additional ref and call the function only one time (after that change ref value to true and if it's true then don't call .focus).

But I'm curious, isn't view supposed to be already laid out in the native side when useEffect is called in the component?

Well, this is my understanding as well (like view should be mounted and laid out) 🤷‍♂️
I have an assumption that TextInput may do its own layout calculation and it can be actually mounted in native view hierarchy, but may require one additional frame to be properly laid out (but it's just an assumption).

I had "similar" issue in #491 - there I also had to miss one frame to properly pick up selection coordinates when multiline input grows.

Maybe a problem (as you pointed out earlier) comes from the fact, that TextInput needs an additional frame to be laid out when parent is currently animating. A lot of guesses - just need time to investigate that. But I think a combination of animation + laying out a view requires an extra frame. And most likely it's not something that I can fix (since it'll come from react-native core).

So I think calling a .focus with delay is a good workaround for now 🙃

@kirillzyusko
Copy link
Owner

Here're related issues:

Read these topics more carefully - well this is really frustrating, that you can not customize handling of multiline input with custom enter behavior 🤯
The Fabric definitely will fix the problem because you can synchronously call functions from JS etc., but right now two architectures are suported, so that means that Meta will not consider this to be fixed until two architectures are supported at the same time.

Let's hope that in 0.76 the Fabric will become a default architecture and then after several month the issue will be closed and TextInputs will have a better customization 🤞

@thisisgit Look what I found: facebook/react-native#33653 Seems like you may have multiline input, call submit function and control from JS whether you should close a keyboard or not 👀

@thisisgit
Copy link
Author

@kirillzyusko This is AWESOME! Thank you so much for finding this! I thought this would be the best solution since it also fixes the keyboard flickering issue. And... you found it!

It was little tricky to find out on how to use the added feature since that submitBehavior prop didn't show up in autocomplete. Also the official documentation didn't include any info about it so I first thought that it isn't released yet.

But it worked just fine when I tried it:

<TextInput
      ref={inputRef}
      style={styles.input}
      defaultValue={text}
      multiline={true}
      // This!
      submitBehavior={'submit'}
      onSubmitEditing={onSubmit}
    />
Simulator.Screen.Recording.-.iPhone.15.-.2024-07-23.at.14.19.25.mp4

Found out that the PR didn't include Typescript support so I created a PR on react-native to support it and hope it gets merged ASAP without any issue. Meanwhile, I'll just suppress the type error.

Again, I really appreciate your help on resolving issues that is out-of-scope of this library 🙏

I'll close this issue as multiple solutions have been suggested.

@kirillzyusko
Copy link
Owner

@thisisgit nice! Would be good to submit a PR to the documentation website as well I think 🙃

New demo from UI perspective looks much better 😍 It looks exactly like it should work!

@thisisgit
Copy link
Author

@kirillzyusko Yup I'll check on the website once this gets merged since there may be something that I'd be missing. Thanks!

@kirillzyusko
Copy link
Owner

@thisisgit I think your PR was already merged 🙃 It was imported on a Phabricator, so now it's only a technical aspect when your PR will update the status on GitHub 👀

@thisisgit
Copy link
Author

@kirillzyusko Oh thanks for letting me know. It's a lot quicker than I thought😮 I'll come back and check the website once I'm done with some urgent tasks. Probably at most this weekend?

@kirillzyusko
Copy link
Owner

@thisisgit yeah, sure, take your time 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component question You wanted to clarify something about the usage of the library or have a question about something
Projects
None yet
Development

No branches or pull requests

2 participants