search

APSL / react-native-keyboard-aware-scroll-view

A ScrollView component that handles keyboard appearance and automatically scrolls to focused TextInput.
MIT License
5.27k stars 646 forks source link

fix RN 0.65 crash #501

Closed oliverdolgener closed 2 years ago

garydevenay commented 3 years ago

Can confirm that this fixes my crash issue on 0.65.1

Great-hijack commented 3 years ago

Can't wait to see the next version available on npmjs

akinolu52 commented 3 years ago

can i pls get a timeline as to when this fix will be merged ?

donnes commented 3 years ago

Please, merge this ASAP! Thanks

mingtsay commented 3 years ago

Tried install using: npm install --save APSL/react-native-keyboard-aware-scroll-view#pull/501/head or yarn add APSL/react-native-keyboard-aware-scroll-view#pull/501/head It works!

This will change your dependency in package.json to:

- "react-native-keyboard-aware-scroll-view": "^0.9.4",
+ "react-native-keyboard-aware-scroll-view": "APSL/react-native-keyboard-aware-scroll-view#pull/501/head",
mzxyz commented 3 years ago

Would be good to have this merged, looks just api changed and all approved. Looking forward to see this include this include in the next release

dongdyang commented 3 years ago

@oliverdolgener please help to merge. Thanks!

mlazari commented 3 years ago

I am seeing this crash only in iOS simulator, works fine on my iPhone. With this change it works on both.

mingtsay commented 3 years ago

I am seeing this crash only in iOS simulator, works fine on my iPhone. With this change it works on both.

It crashes on both simulator and physical devices for me. (perhaps your device has js bundle shipped while building and not updated? I duno.) But this PR certainly fixes that.

kkkorez commented 3 years ago

Please merge @oliverdolgener

mlazari commented 3 years ago

@mingtsay

I am seeing this crash only in iOS simulator, works fine on my iPhone. With this change it works on both.

It crashes on both simulator and physical devices for me. (perhaps your device has js bundle shipped while building and not updated? I duno.) But this PR certainly fixes that.

Yes, you're right. I double checked and it happens on both. The difference is that my real device is iPhone SE 2020 and the iOS simulator I was using was iPhone 12 Pro Max. On SE the error occurs for me when the keyboard is dismissed, while on 12 Pro Max - when it opens. Probably it's because of different screen size or something.

oliverdolgener commented 3 years ago

Please merge @oliverdolgener

I wish I could but I'm just a user like you guys :(

eliw00d commented 3 years ago

@slorber Are you able to approve, merge, and increment the npm version?

mingtsay commented 3 years ago

I go read the source code of React Native 0.48 branch and see this line: https://github.com/facebook/react-native/blob/6ce42441ec98bb8543e8eff8849ce50e076ce520/Libraries/Components/ScrollView/ScrollView.js#L493 So I can confirm that this change will work from 0.48 to 0.65, and also 0.66 (next).

scrollResponderScrollTo is gone because the file ScrollResponder.js in react-native/Libraries/Components/ is removed from branch of 0.65.

Feel free to try this PR in React Native 0.48 to latest version to make sure whether it works or not if you like to.

kutaisan commented 3 years ago

Tried install using: npm install --save APSL/react-native-keyboard-aware-scroll-view#pull/501/head or yarn add APSL/react-native-keyboard-aware-scroll-view#pull/501/head It works!

This will change your dependency in package.json to:

- "react-native-keyboard-aware-scroll-view": "^0.9.4",
+ "react-native-keyboard-aware-scroll-view": "APSL/react-native-keyboard-aware-scroll-view#pull/501/head",

its not working on AppCenter builds. This PR has to be merged

mzxyz commented 3 years ago

Prefer this fix to be merged ASAP, but to whom may need the fix ATM, the alternative way is using patch-package to generate the patch file until the owner release the new version

tonype commented 3 years ago

Posting to also request this be merged ASAP, running into this upgrading to 0.65.

kutaisan commented 3 years ago

I forked repo and published with this PR at npm for AppCenter build.

https://www.npmjs.com/package/kutaisan-react-native-keyboard-aware-scroll-view

You can use this until this PR has been merged (dont forget to change, this won't be get updated)

knro commented 3 years ago

Please merge this whenever you can to fix the crash.

jxom commented 3 years ago

Prefer this fix to be merged ASAP, but to whom may need the fix ATM, the alternative way is using patch-package to generate the patch file until the owner release the new version

Quick note – this is done by doing the following:

  1. Run yarn add patch-package -D
  2. Make the changes from this PR in your node_modules/react-native-keyboard-aware-scroll-view/lib/KeyboardAwareHOC.js
  3. Run yarn patch-package react-native-keyboard-aware-scroll-view to save the patch
  4. Add "postinstall": "patch-package" to your package.json to apply the saved patch on install

https://github.com/ds300/patch-package#readme

hsavit1 commented 3 years ago

updates?

slorber commented 3 years ago

I have permission to merge and publish this package, but can't dedicate time to it.

Who has used the fork, and can confirm it works for both older versions of RN + 0.65?

Merging a change that only works for the new RN version would harm the community. What is the current RN support of current lib version? When was scrollTo introduced exactly?

Please make sure that it's working for you (on RN 0.65) but for others too, and invest your own time to convince me it's the case with a demo app or whatever, because I can't blindly merge this PR nor invest my own time atm.

mingtsay commented 3 years ago

I have permission to merge and publish this package, but can't dedicate time to it.

Who has used the fork, and can confirm it works for both older versions of RN + 0.65?

Merging a change that only works for the new RN version would harm the community. What is the current RN support of current lib version? When was scrollTo introduced exactly?

Please make sure that it's working for you (on RN 0.65) but for others too, and invest your own time to convince me it's the case with a demo app or whatever, because I can't blindly merge this PR nor invest my own time atm.

You might want to read this comment[1] that I post before. Also, scrollTo exists for a long while. At least it works in 0.48 and later.

[1] https://github.com/APSL/react-native-keyboard-aware-scroll-view/pull/501#issuecomment-919625997

CoryWritesCode commented 3 years ago

created a backwards compatible fix complete with newly init'ed apps referenced with the fix implemented #510

alexbuckham commented 2 years ago

Sometimes other dependencies within your project (for example, Native Base) will depend on an older version of the @codler fix. Add "resolutions": { "native-base/@codler/react-native-keyboard-aware-scroll-view": "2.0.0" } to your package.json file and run yarn/npm install.

jgalianoz commented 2 years ago

Please merge 🙏🏼

thatgriffith commented 2 years ago

Tried install using: npm install --save APSL/react-native-keyboard-aware-scroll-view#pull/501/head or yarn add APSL/react-native-keyboard-aware-scroll-view#pull/501/head It works! This will change your dependency in package.json to:

- "react-native-keyboard-aware-scroll-view": "^0.9.4",
+ "react-native-keyboard-aware-scroll-view": "APSL/react-native-keyboard-aware-scroll-view#pull/501/head",

its not working on AppCenter builds. This PR has to be merged

Try adding github: before the path: "react-native-keyboard-aware-scroll-view": "github:APSL/react-native-keyboard-aware-scroll-view#pull/501/head"

SebastianMieszczanczyk commented 2 years ago

Prefer this fix to be merged ASAP, but to whom may need the fix ATM, the alternative way is using patch-package to generate the patch file until the owner release the new version

Quick note – this is done by doing the following:

  1. Run yarn add patch-package -D
  2. Make the changes from this PR in your node_modules/react-native-keyboard-aware-scroll-view/lib/KeyboardAwareHOC.js
  3. Run yarn patch-package react-native-keyboard-aware-scroll-view to save the patch
  4. Add "postinstall": "patch-package" to your package.json to apply the saved patch on install

https://github.com/ds300/patch-package#readme

This is the best solution until they will merge this PR. It fixed the issue in my case. @jxom Thanks

denizkovanci commented 2 years ago

This pull request approved by community. Please, don't wait, merge it.

lfoliveir4 commented 2 years ago

PLEASE MERGE THIS!!!!!!! @RajeshCentrica

r0nho commented 2 years ago

MERGE PLEASE!

mlazari commented 2 years ago

I think this can be closed in favor of https://github.com/APSL/react-native-keyboard-aware-scroll-view/pull/510 which ensures backward compatibility with older RN versions.

slorber commented 2 years ago

This PR does not handle retrocompatibilit, closing in favor of https://github.com/APSL/react-native-keyboard-aware-scroll-view/pull/510