ascoders / react-native-image-zoom

react native image pan and zoom
MIT License
639 stars 282 forks source link

Refactor the image zoom component to avoid the use of deprecated methods #127

Closed TPXP closed 4 years ago

TPXP commented 4 years ago

componentWillMount and componentWillReceiveProps are deprecated lifecycle methods and should be avoided. This PR offers to move the componentWillMount part to the constructor (which has the exact same effect), and moves the componentWillReceiveProps part to componentDidUpdate, which is safe since the function performs comparison between props to detect any change (thus the additional calls will not result in unexpected behaviour).

Diff kinda messed up because I removed one indentation as I moved the pan responder to the constructor.

Fixes #111 and closes #114. After merging this PR, consider releasing a new version of this module as well as https://github.com/ascoders/react-native-image-viewer to fix https://github.com/ascoders/react-native-image-viewer/issues/360

pxpeterxu commented 4 years ago

@TPXP: thanks a ton for putting the work in here! I ran into the same issues.

Might it be possible for this to use static getDerivedStateFromProps rather than componentDidUpdate as the replacement for componentWillReceiveProps? (Performance-wise, this would require only one re-render rather than two)

Edit: I see that the componentDidUpdate() calls centerOn, which is an animation, so it's the right place for it!

Also, it might be a bit easier to review this if you moved the panResponder initialization into a constructor() (by renaming componentWillMount to just

constructor(props: Props) {
  super(props);
  this.panResponder = ...;
}
ArtemKolichenkov commented 4 years ago

@TPXP nice work! I've started some cleaning in this repo though and unfortunately now your branch has conflicts. I refactored the methods in PR #130 and trying to release it ASAP so I will close your PR, sorry.
I will make sure to mark your contributions once I have allcontributors bot installed in this repo (can't do it myself, need @ascoders to do it or make it old-fashioned manual way).

P.S. If you're interested what's going on with this project - read #129

ArtemKolichenkov commented 4 years ago

@all-contributors please add @kingdaro for code

allcontributors[bot] commented 4 years ago

@ArtemKolichenkov

I've updated the pull request to add @kingdaro! :tada:

ArtemKolichenkov commented 4 years ago

oops, copy pasted from other PR 😅 @all-contributors please add @TPXP for code

allcontributors[bot] commented 4 years ago

@ArtemKolichenkov

I've put up a pull request to add @TPXP! :tada: