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

Enabled does not work as expected #414

Closed
flodlc opened this issue Apr 17, 2024 · 8 comments · Fixed by #417
Closed

Enabled does not work as expected #414

flodlc opened this issue Apr 17, 2024 · 8 comments · Fixed by #417
Assignees
Labels
🐛 bug Something isn't working 📚 components Anything related to the exported components of this library KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component

Comments

@flodlc
Copy link

flodlc commented Apr 17, 2024

Hi there,
I use enabled property in case of autofocus of an input in a modal context. I do it to prevent the keyboard opening competing with the modal height transition which leads to broken animation.
I delay the activation of KeyboardAwareScrollView using enabled: false and then true after 1 second of delay.
The animation works but the KeyboardAwareScrollView is not working until I blur the input (keyboard collapse) and reopen it.
Hope it helps :)

To Reproduce
Steps to reproduce the behavior:

  • wrap a form with KeyboardAwareScrollView (enabled: false)
  • set autofocus on first field
  • set enabled true after 1 second

For the delay I use this hook:

const useDelayed = (enabled: boolean) => {
  const [value, setValue] = useState(false);
  useEffect(() => {
    if (!enabled) {
      setValue(false);
      return;
    }
    if (value) return;
    setTimeout(() => setValue(true), 1000);
  }, [enabled]);
  return value;
};

Expected behavior
After enabled turns on, it should work as usual.

Smartphone (please complete the following information):

  • Device: iPhone 15
  • OS: IOS
  • RN version: [e.g. 0.73.6]
  • RN architecture: old
  • JS engine: Hermes
  • Library version: 1.11.6
@kirillzyusko
Copy link
Owner

Hi @flodlc 👋

After enabled turns on, it should work as usual.

May I ask you to provide more input here? What is the exact scenario which is not working? When KeyboardAwareScrollView is disabled the keyboard overlaps a focused input and when you make it enabled it's not avoiding a focused input? Or when it becomes enabled and you change focus - it's not reacting on focus changes?

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 📚 components Anything related to the exported components of this library labels Apr 19, 2024
@flodlc
Copy link
Author

flodlc commented Apr 19, 2024

Hi @kirillzyusko, sure I recorded 2 videos to show you 2 cases of behavior that I have.

On this video, I wrap my form with KeyboardAwareScrollView and I have the first input with autofocus. It competes with the modal animation and leads to a broken modal animation animation (the scroll gets crazy).
https://github.com/kirillzyusko/react-native-keyboard-controller/assets/3781663/7b27f1f2-afc3-484f-958c-1741887b3254

To manage it, I set the KeyboardAwareScrollView enabled=false on mount and set enabled=true only after 1 second.
I expected the KeyboardAwareScrollView to work then, but set enabled is not enough, I have to press out of the input to hide the keyboard and focus again. Then everything work normally.
https://github.com/kirillzyusko/react-native-keyboard-controller/assets/3781663/54cf4b16-58c6-4d86-9121-b7024b732915
Here is how I use it:

const delayedTrue = useDelayed(1000);

return 
  (<KeyboardAwareScrollView
    enabled={delayedTrue}
    bottomOffset={10}
    keyboardShouldPersistTaps="handled"
  >
    {...}
  </KeyboardAwareScrollView>
);

So to resume, the problem is that setting enabled after 1 second does not enables the KeyboardAwareScrollView untill the keyboard hide/show again.
In a perfect world, I would like to achieve to have a modal animation without having to disable KeyboardAwareScrollView.
Maybe @gorhom has thought on this usecase ?

@kirillzyusko
Copy link
Owner

@flodlc cool, I understand the second problem now, thanks for sharing videos ❤️

This is a definetely bug - I'll try to fix it. I think a fix may be similar to #399 but may need to adjust code in KeyboardAwareScrollView as well 👍

Thank you for reporting this problem! I'll try to fix it next week 👍

Maybe @gorhom has thought on this usecase ?

I'm always open for discussions and any suggestions for the code changes in my library 👀 Though i think a problem is a bit tricky to solve (because you are animating a view + change a size of this view). But yeah, anyway, I'm open for discussions and potential ways to resolve the problem 👀

@flodlc
Copy link
Author

flodlc commented Apr 19, 2024

Nice, if you need more details feel free to ask.
Good to here that the react-native-keyboard-controller is well maintained and keep improving 💪

@kirillzyusko
Copy link
Owner

@flodlc may I ask you to test #417 and let me know whether it fixes a problem or not?

@flodlc
Copy link
Author

flodlc commented Apr 20, 2024

It works absolutely how it should !
Thanks a lot for your reactivity 👍

kirillzyusko added a commit that referenced this issue Apr 20, 2024
…AwareScrollView`) (#417)

## 📜 Description

React on `enabled` property toggle when keyboard open in
`KeyboardAwareScrollView` component.

## 💡 Motivation and Context

Initially me and @IvanIhnatsiuk were thinking that in
#350
it will be better to not handle keyboard appearance if `enabled=false`.
But it turns out that it will produce issues like
#414

So in this PR I'm always keeping a handler in `useKeyboardHandler` hook
and instead I don't render additional padding if view is disabled (we
are not making any scrolling because we already handle `enabled`
property value in `maybeScroll`).

Fixes
#414

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- don't put keyboard handlers conditionally;
- make padding conditional depends on `enabled` property;

## 🤔 How Has This Been Tested?

Tested manually on iPhone 15 Pro (iOS 17.4, simulator).

## 📸 Screenshots (if appropriate):

<!-- Add screenshots/video if needed -->
<!-- That would be highly appreciated if you can add how it looked
before and after your changes -->

|Before|After|
|-------|-----|

|![image](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/e37f7dbc-b915-4f6c-993d-7b672672be66)|![image](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/44127abe-4353-4aff-abf3-aa69491e5383)|
|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/54de59f6-4f5e-45cd-ae4b-aa34cea6f56c">|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/4e4fccf0-9c1e-4a94-a12e-17a8412b2244">|


## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko
Copy link
Owner

@flodlc thanks, glad to hear it works!

I may release a new library version next week, but if it's blocking you -> you can apply changes from this PR using a patch-package or consume a version directly from main branch 😊

@kirillzyusko
Copy link
Owner

Published in 1.11.7

@kirillzyusko kirillzyusko added the KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 📚 components Anything related to the exported components of this library KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component
Projects
None yet
2 participants