2

In a React JS component I am rendering a list of items (Recipes), using JS map function from an array, passed in from a App parent component. Each item has a sub list (Ingredients), again rendered using map function.

What I want is to show/hide the sub list of Ingredients when you click on the Recipe title. I use a onClick function on the title that sets the CSS to display none or block, but I get the following error:

Uncaught TypeError: Cannot read property 'openRecipe' of undefined

Here is my code:

var App = React.createClass({


  getInitialState(){

    return{

      showModal:false,


      recipeKeys: [ ],

      recipes: [ ]

    }


  },

  addRecipeKey: function(recipe){


    var allKeys = this.state.recipeKeys.slice();

    var allRecipes = this.state.recipes.slice();

    allKeys.push(recipe.name);

    allRecipes.push(recipe);

    localStorage.setObj("recipeKeys", allKeys);


    this.setState({recipeKeys: allKeys, recipes: allRecipes}, function(){

      console.log(this.state);


    });


  },

  componentDidMount: function(){

    var dummyRecipes = [

      {
        "name": "Pizza",

        "ingredients": ["Dough", "Tomato", "Cheese"]

      },

      {
        "name": "Sushi",

        "ingredients": ["Rice", "Seaweed", "Tuna"]

      }

    ]


    if(localStorage.getItem("recipeKeys") === null){

        localStorage.setObj("recipeKeys", ["Pizza", "Sushi"]);

        dummyRecipes.forEach(function(item){

        localStorage.setObj(item.name, item);  

        });    

        this.setState({recipeKeys: ["Pizza", "Sushi"], recipes: dummyRecipes}, function(){

          console.log(this.state);

        });


    } else {

      var recipeKeys = localStorage.getObj("recipeKeys");

      var recipes = [];

      recipeKeys.forEach(function(item){

        var recipeObject = localStorage.getObj(item);

        recipes.push(recipeObject);

     });


     this.setState({recipeKeys: recipeKeys, recipes: recipes}, function(){

       console.log(this.state);

     });



    }


  }, 


  open: function(){


    this.setState({showModal:true});

  },

    close: function(){


    this.setState({showModal:false});

  },

  render: function(){

    return( 

      <div className="container">
        <h1>Recipe Box</h1>
        <RecipeList recipes = {this.state.recipes} />
        <AddRecipeButton openModal = {this.open}/>
        <AddRecipe closeModal = {this.close} showModal={this.state.showModal} addRecipeKey = {this.addRecipeKey}/>
      </div>

    )  

  }


});

var RecipeList = React.createClass({


  openRecipe: function(item){


    var listItem = document.getElementById(item);

    if(listItem.style.display == "none"){

        listItem.style.display = "block";

    } else {

      listItem.style.display = "none";      
    }    

   },

  render: function(){    

   return (

      <ul className="list-group">

        {this.props.recipes.map(

          function(item,index){

            return (

              <li className="list-group-item" onClick={this.openRecipe(item)}> 
                <h4>{item.name}</h4>
                <h5 className="text-center">Ingredients</h5>
                <hr/>
                <ul className="list-group" id={index} >
                {item.ingredients.map(function(item){

                  return (
                    <li className="list-group-item">  
                    <p>{item}</p>
                     </li>  
                  )

                })} 
                </ul>  
              </li>  

            )

          }


        )


        }

      </ul>  


    ) 


  } 


});

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

Also, I am trying to use a CSS method here, but maybe there is a better way to do it with React?

Can anyone help me? Thanks!

chemook78
  • 1,168
  • 3
  • 17
  • 38
  • Doesn't DOM access/manipulation defeat the purpose of React. Never a javascript select in my react code. Usually change state and have the state change a css className via a ternary... – Michael Paccione Dec 13 '16 at 04:38
  • Yes you are right I think, trying to implement it using React state now – chemook78 Dec 13 '16 at 06:33

2 Answers2

2

your issue is you are losing your this context in your map... you need to add .bind(this) to the end of your map function

{this.props.recipes.map(function(item,index){...}.bind(this))};

I answered another question very similar to this here. If you can use arrow functions it auto binds for you which is best. If you can't do that then either use a bind or make a shadow variable of your this context that you use inside the map function.

Now for the cleanup part. You need to clean up your code a bit.

var RecipeList = React.createClass({
  getInitialState: function() {
    return {display: []};
  },
  toggleRecipie: function(index){
    var inArray = this.state.display.indexOf(index) !== -1;

    var newState = [];
    if (inArray) { // hiding an item
        newState = this.state.display.filter(function(item){return item !== index});
    } else { // displaying an item
        newState = newState.concat(this.state.display, [index]);
    }
    this.setState({display: newState});
  },
  render: function(){
   return (
      <ul className="list-group">
        {this.props.recipes.map(function(item,index){
            var inArray = this.state.display.indexOf(index) !== -1;
            return (
              <li className="list-group-item" onClick={this.toggleRecipie.bind(this, index)}> 
                <h4>{item.name}</h4>
                <h5 className="text-center">Ingredients</h5>
                <hr/>
                <ul className="list-group" id={index} style={{display: inArray  ? 'block' : 'none'}} >
                {item.ingredients.map(function(item){
                  return (
                    <li className="list-group-item">  
                    <p>{item}</p>
                     </li>  
                  )
                }.bind(this))} 
                </ul>  
              </li>  
            )
          }.bind(this))
        }
      </ul>  
    ) 
  } 
});

This may be a little complicated and you may not want to manage a list of indicies to toggle a view of ingredients. I'd recommend you make components for your code, this way its more react centric and it makes toggling a view much easier.

I'm going to write this in ES6 syntax also as you should be using ES6.

const RecipieList = (props) => {
    return (
        <ul className="list-group">
            {props.recipes.map( (item,index) => <RecipieItem recipie={item} /> )
        </ul>  
    );
};

class RecipieItem extends React.Component {
    constructor(){
        super();
        this.state = {displayIngredients: false}
    }
    toggleRecipie = () => {
        this.setState({displayIngredients: !this.state.displayIngredients});
    }
    render() {
        return (
            <li className="list-group-item" onClick={this.toggleRecipie}> 
                <h4>{item.name}</h4>
                <h5 className="text-center">Ingredients</h5>
                <hr/>
                <ul className="list-group" style={{display: this.state.displayIngredients  ? 'block' : 'none'}} >
                    {this.props.recipie.ingredients.map( (item) => <IngredientItem ingredient={item} /> )} 
                </ul>  
            </li>
        );
    }
}

const IngredientItem = (props) => {
    return (
        <li className="list-group-item">  
            <p>{props.ingredient}</p>
        </li>
    );
};
Community
  • 1
  • 1
John Ruddell
  • 25,283
  • 6
  • 57
  • 86
  • Hi John, thanks! I get the following error when using bind like you said: _Uncaught TypeError: this.props.recipes.map(...).bind is not a function_ – chemook78 Dec 13 '16 at 04:46
  • 1
    @chemook78 im sorry I just edited, the .bind(this) needs to be on the function, not the map :) aka map(function(){}.bind(this) ) – John Ruddell Dec 13 '16 at 04:48
  • 1
    You could also use ES6 arrow functions. `array.map(elem => /* do stuff */)` – 0xcaff Dec 13 '16 at 06:05
  • @caffinatedmonkey not only do I recommend inline lambdas (fat arrow functions) I have my linter require them... but the code example the OP provided is in ES5, so I wrote an ES5 solution :( If you look at the link I added in the answer you'll see I use fat arrows there :) – John Ruddell Dec 13 '16 at 06:09
  • @JohnRuddell thanks for helping with the cleaner code. It makes _all_ recipes show up or hide though. I want a specific recipe to show up once you click on the title actually...any suggestions how I can do that using React state? – chemook78 Dec 13 '16 at 06:32
  • @chemook78 see my latest edit. I changed state to be an array so you can keep track of which item is clicked on – John Ruddell Dec 13 '16 at 06:39
  • @chemook78 also just edited with an alternate (better) solution you should give a shot – John Ruddell Dec 13 '16 at 07:34
  • @JohnRuddell thanks! Will implement that. Something about the previous code I don't understand is: why do we need the bind(this) call on the function in *{this.props.recipes.map(function(item,index)...*? Where does this refer to here? – chemook78 Dec 13 '16 at 08:09
  • 1
    in javascript EVERY function has a this context. inside your map function the THIS context is completely different to the react context. so this.state is undefined. `.bind(this)` puts the react component THIS context on the function which makes the rest of your code work – John Ruddell Dec 13 '16 at 08:11
  • @JohnRuddell yeah, definitely should re-write it in components just like you did! – chemook78 Dec 13 '16 at 08:19
0

You also can use something like this:

  render: function(){    
   var self = this;
   return (
      <ul className="list-group">
        {this.props.recipes.map(
          function(item,index){
            return (
              <li className="list-group-item" onClick={self.openRecipe(item)}>.....
Vitalii Andrusishyn
  • 3,984
  • 1
  • 25
  • 31