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

Android: scroll disabled using adjustToContentHeight #219

Open
danilocanalle opened this issue May 27, 2020 · 24 comments
Open

Android: scroll disabled using adjustToContentHeight #219

danilocanalle opened this issue May 27, 2020 · 24 comments

Comments

@danilocanalle
Copy link

danilocanalle commented May 27, 2020

Describe the bug
Hello,
You have fixed other bug that I have related to iOS, similar one.
But now, Iam testing on Android and I went to a bug here.

If we use adjustToContentHeight, the content ins't scrolling.

Another bug that you can reproduce in the same snack bellow is:
We cant drag the modal clicking on the HeaderComponent (ios does). If you want, I can open a new issue with that one.
Edit: #223

Reproduce
https://snack.expo.io/@bookplay/react-native-modalize-bug-scroll

Dependencies:

  • react-native-modalize [2.0.4]
@ivanliut
Copy link

ivanliut commented Jun 4, 2020

I have experienced the same issue with adjustToContentHeight and disable scrolling for Android. Could you please fix this issue asap, that'll be extremely helpful.

@ydv0121
Copy link

ydv0121 commented Jun 4, 2020

for android and ios both side scroll not working if we enable adjustToContentHeight

@danilocanalle
Copy link
Author

for android and ios both side scroll not working if we enable adjustToContentHeight

For me , on IOS, its ok since the last version that he fixed it.

@danilocanalle
Copy link
Author

I have experienced the same issue with adjustToContentHeight and disable scrolling for Android. Could you please fix this issue asap, that'll be extremely helpful.

A workaround till it be fixed is:
disableScrollIfPossible={false}

So, we always be with scroll enabled.

Of course, just a workaround till we have a fix for it.

Thanks, waiting for @jeremybarbet the best! haha

@ivanliut
Copy link

ivanliut commented Jun 5, 2020

@danilo900 You've made my day! Thanks for your help

@ydv0121
Copy link

ydv0121 commented Jun 5, 2020

@danilo900 Thank you for the workaround..it works only if we didn't use alwaysOpen.
but in my case i am using alwaysOpen and put this disableScrollIfPossible={false}.
So it's not working when i am using alwaysOpen. ☹️

@jeremybarbet

@ydv0121
Copy link

ydv0121 commented Jun 8, 2020

@danilo900 do you have any other workaround to make scorll work when using alwaysOpen

@danilocanalle
Copy link
Author

@danilo900 do you have any other workaround to make scorll work when using alwaysOpen

Sorry, I don't, Iam not using alwaysopen =/

@ydv0121
Copy link

ydv0121 commented Jun 14, 2020

@jeremybarbet is there any possibilities to solve this???

@fadiaIW
Copy link

fadiaIW commented Aug 1, 2020

I have experienced the same issue with adjustToContentHeight and disable scrolling for Android. Could you please fix this issue asap, that'll be extremely helpful.

A workaround till it be fixed is:
disableScrollIfPossible={false}

So, we always be with scroll enabled.

Of course, just a workaround till we have a fix for it.

Thanks, waiting for @jeremybarbet the best! haha

Thanks! this solved my problem!

@jeremybarbet
Copy link
Owner

Hey,

Could you try to install yarn add "git://github.com/jeremybarbet/react-native-modalize.git#bebd6ec2621980b11290ec55cc23f988a264dfdd" I will publish a new version if it's working

@danilocanalle
Copy link
Author

Hello @jeremybarbet
Did you already published that?

@danilocanalle
Copy link
Author

danilocanalle commented Sep 14, 2020

I haved tried your last commit, 2.0.6
No success at all =/

Sorry for the late answer, I was off of the project, Iam back now to help testing.

@otaviobonder-deel
Copy link

Still not working though

@swingywc
Copy link

Hi @jeremybarbet,
I have faced the same issues as @danilo900 and have made a PR for fixing it.
If you have time, could you please help to look into it and see if this is good to be released to production? Thank you.

@swingywc
Copy link

For anyone who is using patch-package and waiting for the fix, please help to try the below patch and see if it has any issue:

File version name: react-native-modalize+2.0.8.patch

diff --git a/node_modules/react-native-modalize/lib/index.js b/node_modules/react-native-modalize/lib/index.js
index 28e86cd..ffc3efc 100644
--- a/node_modules/react-native-modalize/lib/index.js
+++ b/node_modules/react-native-modalize/lib/index.js
@@ -275,19 +275,15 @@ onOpen, onOpened, onClose, onClosed, onBackButtonPress, onPositionChange, onOver
         const shorterHeight = maxFixed < endHeightFixed;
         setDisableScroll(shorterHeight && disableScrollIfPossible);
     };
-    const handleContentLayout = ({ nativeEvent }) => {
-        if (onLayout) {
-            onLayout(nativeEvent);
-        }
+    const handleContentLayout = (width, height) => {
         if (alwaysOpen && adjustToContentHeight) {
-            const { height } = nativeEvent.layout;
             return setModalHeightValue(height);
         }
         // We don't want to disable the scroll if we are not using adjustToContentHeight props
         if (!adjustToContentHeight) {
             return;
         }
-        handleBaseLayout('content', nativeEvent.layout.height);
+        handleBaseLayout('content', height);
     };
     const handleComponentLayout = ({ nativeEvent }, name, absolute) => {
         /**
@@ -531,7 +527,7 @@ onOpen, onOpened, onClose, onClosed, onBackButtonPress, onPositionChange, onOver
                 listener: onScrollBeginDrag,
             }),
             scrollEventThrottle,
-            onLayout: handleContentLayout,
+            onContentSizeChange: handleContentLayout,
             scrollEnabled,
             keyboardDismissMode,
         };

@danilocanalle
Copy link
Author

Hello @swingywc,
I have tested your patch, its working perfect!

Thank you so much for your time :)

@vibern0
Copy link

vibern0 commented Aug 19, 2021

@swingywc solution is working for me with expo sdk42 and almost all packages with the most recent version.
@impactMarket is thankful for your contribution 🙏

@wolframus
Copy link

wolframus commented Nov 12, 2021

const { height } = Dimensions.get('window')

 const [contentHeight, setContentHeight] = useState(0)

return (
<Modalize adjustToContentHeight={height * 0.9 > contentHeight} {...otherProps}>
  <View  onLayout={({ nativeEvent }) => setContentHeight(nativeEvent.layout.height)}>
    // here goes your scrollable content of modalize
  </VIew>
</Modalize>
)

@doougui
Copy link

doougui commented Dec 16, 2021

I have experienced the same issue with adjustToContentHeight and disable scrolling for Android. Could you please fix this issue asap, that'll be extremely helpful.

A workaround till it be fixed is: disableScrollIfPossible={false}

So, we always be with scroll enabled.

Of course, just a workaround till we have a fix for it.

Thanks, waiting for @jeremybarbet the best! haha

Thanks. It worked for me :)

@sallar
Copy link

sallar commented Apr 25, 2022

The patch worked on 2.0.13 too. thank you

@irmakcosarsahna
Copy link

irmakcosarsahna commented Oct 31, 2022

Same issue - 2.1.1 version

@natemartins
Copy link

natemartins commented Dec 14, 2022

Same issue - 2.1.1 version

Looks like this issue has not been fixed for more than 2 years.

The workarounds work if only one modal is open. However, when a modal is opened from an already opened modal, the scroll stops working.

Another issue I noticed is that the second or subsequent modals take the height of the first modal opened even if the content of the subsequent modals is more than the first opened modal.

@jeremybarbet and others, please take a look.

Thanks.

@natemartins
Copy link

natemartins commented Dec 14, 2022

Applying the workaround by @wolframus to the second/subsequent modals restores the scroll but brings back the TypeError: Cannot read property 'layout' of null warning.

The subsequent modals still inherit the height of the first modal.

I could ignore this for now but then what could be the side effect of this warning in production?

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

No branches or pull requests