0

I have a component which displays images on click of a button. These images when get displayed on the screen, I want to apply an onClick listener to them such that the image that I click on gets displayed on the whole screen.

Code:

class App extends React.Component{
  constructor(props){
    super(props);
    this.state={
      images:[],
      pano:'',
      name:'',
      list:[]
    }
    this.loadImages=this.loadImages.bind(this);
    this.loadOne=this.loadOne.bind(this);
  }


  loadImages(){
    console.log("load");
    var that=this;
    $.ajax({
      type:'GET',
      url:'https://demo0813639.mockable.io/getPanos',
      success:function(result){
        var images=that.state.images;
        for(var i=0;i<result.length;i++){
          that.state.images.push({"pano":result[i].pano,"name":result[i].name});
        }
        that.setState({
          images:images
        })
      }
    })
  }

  loadOne(url){
     console.log("hi")
  }


  render(){  
    var list=this.state.list;
    list=this.state.images.map(function(result){
      //console.log(result.name);
      return(<div className="box">
        <div className="label">{result.name}</div>
            <img src={result.pano} className="image"/>   
        </div>
      )
    })
    return( 
      <div>
        <button onClick={this.loadImages}>Click</button>
        <div onClick={this.loadOne(this,this.props.result.pano)}>{list}</div>      
      </div>
    );
  }
}

loadOne() is the function which gets called after clicking on an image. I get the error:

cannot read property pano of undefined

And if I do like this:

render(){
  var list=this.state.list;
  list=this.state.images.map(function(result){
    return(<div className="box">
        <div className="label">{result.name}</div>
          <img src={result.pano} className="image" onClick={this.loadOne(this,this.props.result.pano)}/>   
      </div>
    )
  })

  return( 
    <div>
      <button onClick={this.loadImages}>Click</button>
      <div >{list}</div>      
    </div>
  );
}

then I get the error:

cannot read property loadOne of undefined.

So, how can I pass specific image url to my function?

Mayank Shukla
  • 100,735
  • 18
  • 158
  • 142
Aayushi
  • 1,736
  • 1
  • 26
  • 48
  • `onClick={() => this.loadOne(this,this.props.result.pano)}`. You need to pass a function instead of what you're doing now which is to immediately invoke it during render. I'm sure there's a duplicate for this somewhere, will try to find it. – ivarni Aug 01 '17 at 10:16
  • ^ accepted answer on that also suggest a cleaner approach with sub-components. – ivarni Aug 01 '17 at 10:17
  • Using the above suggestion also, I get cannot read property loadOne of undefined. @ivarni – Aayushi Aug 01 '17 at 10:18
  • @Aayushi you forgot to use `bind` word: `onClick={this.loadOne.bind(this,this.props.result.pano)}` – Mayank Shukla Aug 01 '17 at 10:20
  • even with the bind keyword, I get the same error @MayankShukla – Aayushi Aug 01 '17 at 10:22

2 Answers2

4

Issue with second snippet is:

1- You forgot to bind the map callback method, use arrow function:

var list = this.state.list;
list = this.state.images.map((result) => {   //here
    .....

2- You missed the bind word here:

onClick = {this.loadOne.bind(this,result.pano)}

3- Instead of this.props.result.pano it should be result.pano.

Full code:

var list = this.state.list;
list = this.state.images.map((result) => {
    return(
        <div className="box">
            <div className="label">{result.name}</div>
            <img src={result.pano} className="image" onClick={this.loadOne.bind(this,result.pano)}/>   
        </div>
    )
})

Working Code:

class App extends React.Component{
  constructor(props){
    super(props);
    this.state={
      images:[],
      pano:'',
      name:'',
      list:[]
    }
    this.loadImages=this.loadImages.bind(this);
    this.loadOne=this.loadOne.bind(this);
  }
 
 
  loadImages(){
    var that=this;
    $.ajax({
      type:'GET',
      url:'https://demo0813639.mockable.io/getPanos',
      success:function(result){
        var images=that.state.images;
        for(var i=0;i<result.length;i++){
          that.state.images.push({"pano":result[i].pano,"name":result[i].name});
        }
        that.setState({
          images:images
        })
      } 
    })
  }
 
  loadOne(pano){
    console.log('pano', pano);
  }
 
 
  render(){
    var list = this.state.list;
    list = this.state.images.map((result)=>{
    return(
        <div className="box">
          <div className="label">{result.name}</div>
            <img src={result.pano} className="image" onClick={this.loadOne.bind(this,result.pano)}/>  
        </div>
      )
    })
    return(
      <div>
        <button onClick={this.loadImages}>Click</button>
        <div >{list}</div>      
      </div>
    );
  }
}
 
ReactDOM.render(<App/>,document.getElementById('container'));
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>

<div id='container'/>

Check this answer: Why is JavaScript bind() necessary?

Mayank Shukla
  • 100,735
  • 18
  • 158
  • 142
0

You need to bind you click function to that image.

    class App extends React.Component {
        constructor(props) {
            super(props);
            this.state = {
                images: [],
                pano: '',
                name: '',
                list: []
            }
            this.loadImages = this.loadImages.bind(this);
            //The loadOne cant be bound to this here, it needs a 
            //bind for each image. Effectivly creating one version of loadOne per image.  
        }

        loadImages() {
            $.ajax({
                type: 'GET',
                url: 'https://demo0813639.mockable.io/getPanos',
                success:(result) => {
                    var images = this.state.images;
                    for (var i = 0; i < result.length; i++) {
                        this.state.images.push({ url: result[i].pano, name: result[i].name });
                    }
                    this.setState({
                        images: images
                    })
                }

            })
        }

        loadOne(url, mouseEvent) {
          console.log(url);
        } 


        render() {
            //This can get extracted to a custom component
            var imagesList = this.state.images.map((image, i) => {
                return (<div key={i} className="box" >
                    <div className="label">{image.name}</div>
                    {/*This is where the bind/curry should be */}
                    <img onClick={this.loadOne.bind(this, image.url)} src={image.url} className="image" />
                </div>
                )
            });

            return (
                <div>
                    <button onClick={this.loadImages}>Click</button>
                    <div>{imagesList}</div>
                </div>
            );
        }
    }

ReactDOM.render(<App />, document.getElementById('container'));

Your clickhandler (loadOne) will then be called with the result.pano and the event as second argument passed in by react.

http://jsbin.com/zokugitopa/1/edit?js,output

Per Svensson
  • 751
  • 6
  • 14