marudy / react-native-responsive-screen

Make React Native views responsive for all devices with the use of 2 simple methods
MIT License
1.56k stars 142 forks source link

Fix listener in hooks #70

Open hoanglam10499 opened 4 years ago

hoanglam10499 commented 4 years ago

Solved Issues #69. Please merge

hoanglam10499 commented 4 years ago

Solved Issues #69. Please merge

Cmion commented 4 years ago

I think using a callback for the listenToOrientationChange would be a better choice. And you seem to be re-writing most of the file. I guess it's your formatter.

so consider something like.

`

if (typeof callback === 'function' ){
  const orientation = screenWidth < screenHeight ? 'portrait' : 'landscape';
  callback(orientation);

};`

Lesser code and solves the problem for functional and class based components.

Try refactoring your code, and make changes only to the lines that concerns your PR.

hoanglam10499 commented 4 years ago

@Cmion I didn't change all the files, it was a change of the format code. You can see the "File Changed".

Screen Shot 2020-05-22 at 08 47 26

Cmion commented 4 years ago

Fix listener in hooks by hoanglam10499 · Pull Request #70 · marudy_react-native-responsive-screen - Google Chrome 5_22_2020 18_23_46 Fix listener in hooks by hoanglam10499 · Pull Request #70 · marudy_react-native-responsive-screen - Google Chrome 5_22_2020 18_24_02 Fix listener in hooks by hoanglam10499 · Pull Request #70 · marudy_react-native-responsive-screen - Google Chrome 5_22_2020 18_24_10

I'm not a maintainer, contributor or anything. But It seems like your recent commit made those changes.

Cmion commented 4 years ago

I also looks like the library needs some maintenance too

hoanglam10499 commented 4 years ago

@Cmion It was a change of the format code ...

gregfenton commented 4 years ago

Some quick, hopefully constructive, feedback on this PR:

Thank you for your efforts on this! Really, really appreciated.