douglasjunior / react-native-keyboard-manager

âš› Library to prevent issues of keyboard sliding up and cover inputs on React-Native iOS projects.
https://www.npmjs.com/package/react-native-keyboard-manager
MIT License
971 stars 59 forks source link

Update IQKeyboardManager to latest version #65

Closed zoontek closed 4 years ago

zoontek commented 4 years ago

Following https://github.com/douglasjunior/react-native-keyboard-manager/issues/64

I updated IQKeyboardManager to v6.5.4 (latest one), but also cleaned up the project a bit (lots of unused dependencies, prettier was only applied on the /Sample project).

Changes

PS: Don't fear the number of lines, it's principally the new IQKeyboardManager folder 😅

douglasjunior commented 4 years ago

Hi @zoontek, thanks for the contribution.

I will calmly review the changes in the late afternoon and give you feedback.

Initially I noticed some things that I don't think are cool, like the unnecessary changes in the README and the addition of prettier in the sample project.

Unnecessary changes increase the complexity of the review, and the prettier will be ignored the next time I update the sample project.

About the hack, it was necessary due to a bug in React Native that prevented the replacement of InputAcessoryVew. Are you sure it is no longer needed?

zoontek commented 4 years ago

@douglasjunior I will remove prettier totally then, and revert the formatting changes.

As I understand, the hack was to override invalidateInputAccessoryView, a method removed in RN 0.55.

The fix in https://github.com/facebook/react-native/issues/16071 has been shipped and I saw absolutely no changes in the module behavior.

douglasjunior commented 4 years ago

Even with the modifications of facebook/react-native#16071, there was still a condition where the invalidateInputAccessoryView was replaced.

If I remember, it's on the numeric keypad, but I need to check.

zoontek commented 4 years ago

@douglasjunior I rolled back a lot of changes, updated the hack to replace setDefaultInputAccessoryView (https://github.com/facebook/react-native/blob/0.60-stable/Libraries/Text/TextInput/RCTBaseTextInputView.m#L561)

When autoToolbar is enabled, InputAccessoryView is now disabled.

todorone commented 4 years ago

@douglasjunior @zoontek Hey guys, is there something, that can be done to push forward this PR?

douglasjunior commented 4 years ago

I still haven't got time to test this new version.

If you guys can test and report the experience here, we can follow up.

I can release a version like "react-native-keyboard-manager@next" if it helps.

douglasjunior commented 4 years ago

This change represents a major update in IQKeyboardManager, so there are some questions to be answered:

douglasjunior commented 4 years ago

I think the IQKeyboardManager files can be removed from the project and then used only via CocoaPods, here https://github.com/douglasjunior/react-native-keyboard-manager/blob/master/ReactNativeKeyboardManager.podspec

douglasjunior commented 4 years ago

I'm going to merge this PR into iqkeyboardmanager-update branch and then we'll talk about this update on issue #68.

todorone commented 4 years ago

@douglasjunior Thanks for pushing forward! Yeah, i guess publishing react-native-keyboard-manager@next version will help everybody to test easily the new version. And yeah, cocoapods dependence without actual native libs inside library is the way to go for common RN dependencies nowadays...

douglasjunior commented 4 years ago

Released react-native-keyboard-manager@alpha.

Any suggestions and bug report please refer to #68.