search

naoufal / react-native-accordion

An Accordion Component for React Native
https://www.npmjs.com/package/react-native-accordion
437 stars 101 forks source link

Slightly hacky support for android - Updated #18

Open adamrainsby opened 8 years ago

adamrainsby commented 8 years ago

My last pull request had a bug. This one should work.

https://github.com/naoufal/react-native-accordion/issues/16

gr4yscale commented 8 years ago

Hi @adamrainsby, can you please explain what the thinking is behind these changes? I see you added the empty onLayout but not sure what the purpose is, is this still a work in progress or does it work with these changes?

adamrainsby commented 8 years ago

@gr4yscale See this issue for the onLayout thing. https://github.com/facebook/react-native/issues/3282

adamrainsby commented 8 years ago

If you want to contribute and get it working with the animation again that would be great. At the moment it literally just shows and hides the content without any animation so it definitely can be improved.

naoufal commented 8 years ago

Hey guys, thanks for moving this forward.

I would opt for a solution that retains the animations. @gr4yscale Have you had any luck fixing this?

adamrainsby commented 8 years ago

@naoufal Just to be clear, the animations still work on iOS but it just opens and closes on android. That is an improvement on it not working at all on android.

I think it should be merged in and then we can open an issue for animations on android?

naoufal commented 8 years ago

@adamrainsby you're right. Before merging, could you post a video of the example running on Android?

adamrainsby commented 8 years ago

@naoufal I had to do a bit of work to get the example app running on android. I've updated my PR with those changes.

iOS specific components like NavigatorIOS were stripped out for the android version but they are still there when running on iOS. I also updated the version of react native to the latest version because there was a bug where the background would disappear after pressing the accordion.

Not sure the exact version this was fixed but for now we can say there is android support from 0.23?

accordion

adamrainsby commented 8 years ago

@naoufal Is this ok?

cfournel commented 8 years ago

@naoufal if this is okay , could you please merge the code and do a release ? i can t use your great module at the moment since my app has to be android / ios compatible. Thanks you !

GeoffreyPlitt commented 8 years ago

+1

naoufal commented 8 years ago

@adamrainsby Sorry for being MIA all. @adamrainsby If you rebase this branch, I'll gladly merge it.

cfournel commented 8 years ago

@naoufal : Here is my test of @adamrainsby . Since i have copied directly @adamrainsby fork into my node_modules dir , i had to install react-tween-state and everything works fine ( except animations of course, but may that could be related to a tween effect or duration ? ) .. here is a screen cast :

adamrainsby commented 8 years ago

@huitiemesens You can put "adamrainsby/react-native-accordion" instead of a version number in the package.json and npm install as usual instead of copy and pasting.

cfournel commented 8 years ago

@adamrainsby thanks for this tips, i m more a composer guy , i didn t know that could be done within a package file

parveen-wizni commented 8 years ago

hi, I am facing some issue. I am using "react-native-accordion": "0.2.4" library for my project. This is working fine in IOS. when I run my app in android device in debugging mode. Then app is running fine. But if I used signed apk in same device then it is give this error. here I paste my application crash report log which are shown in android monitor. Error given below.

FATAL EXCEPTION: mqt_native_modules Process: com.ampsmobile, PID: 2139 com.facebook.react.modules.core.JavascriptException: Requiring unknown module "react-native-accordion", stack: t@2:387 i@2:177

@451:139 t@2:511 i@2:177 @449:139 t@2:511 i@2:177 @448:203 t@2:511 i@2:177 @447:212 t@2:511 t@2:240 i@2:177 component@44:489 u@44:130 o@44:48 createScene@44:1781 _renderScene@244:15724 @244:16158 render@244:16032 _renderValidatedComponentWithoutOwnerOrContext@152:5072 _renderValidatedComponent@152:5200 _updateRenderedComponent@152:4595 _performComponentUpdate@152:4395 updateComponent@152:3672 performUpdateIfNecessary@152:3193 performUpdateIfNecessary@132:667 r@130:571 perform@135:494 perform@135:494 perform@130:1539 P@130:1711 closeAll@135:1131 perform@135:581 batchedUpdates@129:456 s@130:761 a@149:69 enqueueSetState@149:917 setState@29:339 push@244:13537 onPressRow@429:2254 onPress@437:2706 touchableHandlePress@295:1171 _performSideEffectsForTransition@219:8188 _receiveSignal@219:6697 touchableHandleResponderRelease@219:4183 a@113:72 o@111:392 a@111:583 f@109:163 g@109:266 i@115:88 processEventQueue@109:1386 s@163:92 handleTopLevel@163:182 _receiveRootNodeIDEvent@162:617 receiveTouches@162:999 value@60:3582 @60:1801 k@60:436 value@60:1773
cfournel commented 7 years ago

In order to keep it clean @naoufal could you please merge this one ? Thanks

adamrainsby commented 7 years ago

Apologies @huitiemesens, I still need to rebase. I'll try to do it soon.

cfournel commented 7 years ago

@adamrainsby don t worry i already switch for your fork in my package.json , since @naoufal seems afk for a while now ...

In order to fix a great bug ( avoiding to compile a signed android apk ) could you please take a look at https://github.com/naoufal/react-native-accordion/pull/33/commits ( its basically renaming index.ios.js to index.js ) ! Thanks you !

adamrainsby commented 7 years ago

@huitiemesens I already did that in my fork. Are you still having issues with it?

adamrainsby commented 7 years ago

@naoufal Rebased, could you merge?