st0ffern / react-native-image-cropper

Crop your images with zoom and pan
https://www.npmjs.com/package/react-native-image-cropper
MIT License
158 stars 67 forks source link

zoom being ignored on cropper rerender/update #42

Open victorbadila opened 7 years ago

victorbadila commented 7 years ago

Suppose you have

<ImageCrop
          image={this.props.image}
          cropHeight={this.props.height}
          cropWidth={this.props.width}
          zoom={this.state.zoom}
        />

Changing the zoom in the state via .setState does not control the zooming of the cropper. For example I would like to have an "update" action in which I change the image source and I reset the zoom to 0. It works for changing the image source, but providing a new zoom doesn't seem to do anything, the new image is zoomed in as the old image was before updating.

st0ffern commented 7 years ago

Please provide full component. i can see that zoom is from the state of the current component as the rest are props. This is a bug in your component/state.

victorbadila commented 7 years ago

Hi, it is true that the zoom is from the state and the rest are props, why is it a bug? I can't provide the full component, but I will provide a watered down version of it with the essential parts:

Basically when componentWillReceiveProps gets called I can see I have a new image. I also call set state, although maybe I could have zoom as part of the props as well, should behave the same in my opinion though. When rendering with a new image I can see that the state's zoom is once again 0 and since the cropper is "recreated" after that I expect it to take zoom 0 (thus reset whatever zoom was done previously).

On another note it seems that setting the initial zoom to another value (100 for example) still renders the image as if zoom 0 was provided. Maybe I'm missing something here. It happens on iOS 11.

type State = { ... }
type Props = { ... }
class CropTool extends React.Component<State, Props> {
  state = { zoom: 0 };

  componentWillReceiveProps(nextProps) {
    console.log('componentdidupdate', nextProps);
    if (this.props.image !== nextProps.image) {
      console.log('setting state');
      this.setState({ zoom: 0 });
    }
  }
  onBtnPressed = () => {
      return this.cropper.crop()
        .then((base64Data) => {
          // etc
        });
  };

  cropper: { crop: () => Promise<any> };

  render() {
    console.log('rerendering. state, cropper = ', this.state, this.cropper);
    let cropper = null;
    if (this.props.image) {
      cropper =
        (<ImageCrop
          ref={(c) => { this.cropper = c; }}
          zoom={this.state.zoom}
          image={this.props.image}
          cropHeight={this.props.height}
          cropWidth={this.props.width}
          panToMove
          pinchToZoom
        />);
    }
    return (
      <View>
        {cropper}
      </View>
    );
  }
}
st0ffern commented 7 years ago

I dont have the time to debug this for you. It works fine in the simple example, so it is somehow in your component.

victorbadila commented 7 years ago

hi again, I have written an even simpler example that shows this buggy behaviour, plus a possible, yet not pretty, workaround. I will post it here:

import * as React from 'react';
import { View, Button } from 'react-native';
import EStyleSheet from 'react-native-extended-stylesheet';
import { ImageCrop } from 'react-native-image-cropper';

const styles = EStyleSheet.create({
  container: {
    height: 500,
    width: '100%',
    alignItems: 'center'
  },
});

class Cropper extends React.Component {
  // initial state
  state = { zoom: 0 };

  resetZoom = () => {
    this.setState({zoom: 0});
  };

  render() {
    return (
      <View style={styles.container}>
        <ImageCrop
          zoom={this.state.zoom}
          image={"https://i.imgur.com/tCatS2c.jpg"}
          cropHeight={300}
          cropWidth={400}
        />
        <Button
          title="Reset zoom"
          onPress={this.resetZoom}
        />
      </View>
    );
  }
}

export default Cropper;

Zooming normally works with touch behaviour, but when trying to reset the zoom via the button it doesn't do anything, no warnings etc. The only way I was able to achieve this was by forcefully re-rendering the ImageCrop component. I would give the component a key={someId} and would change that whenever I intend to reset the zoom, like for example in the resetZoom method. I think it is an antipattern though, RN complains with warnings that I should not do that.

also, @stoffern unfortunately I did not find the example in this repository that you mentioned to work as expected.

st0ffern commented 6 years ago

did you solve this?

victorbadila commented 6 years ago

hi @stoffern I only was able to solve it via the workaround involving giving a key to the component. the key would change when I want to reset the zoom, which would achieve that by re-rendering the whole ImageCrop component.

st0ffern commented 6 years ago

well that is just a workaround..

has something to do with this: https://github.com/stoffern/react-native-image-cropper/blob/master/src/ImageCrop.js#L159-L163

victorbadila commented 6 years ago

then I conclude I wasn't able to solve it

st0ffern commented 6 years ago

will look into it when i have the time.. If you can provide an example repo that is not working, it would be nice