Kunzite - Barbara

[Barbara-Bennett]

No description provided.

[anselrognlie]

🎉 Nice job! React is challenging and can take several projects to feel comfortable with. I've left comments throughout, so please take a look and let me know in Slack if there's anything you'd like to follow up on.

[anselrognlie]

👀 The value for heart doesn't need to be (and shouldn't be) stored in a local piece of state. One of the values passed in the props is the liked value, and that value (coming all the way from the state stored in App) gets updated when the like button is clicked. We can calculate which heart emoji to display as follows:

  const heart = props.liked ? '❤️' : '🤍';

We don't need a full piece of state here, since the liked value is tracked in state above this component. We should avoid duplicating the value here in state. Instead, we can just calculate a local variable (it will be calculated on each render), or even embed the emoji calculation in the button markup itself (though I do like storing it in a local variable)!

        <button className="like" onClick={onLikeButtonClick}>{props.liked ? '❤️' : '🤍'}</button>

[anselrognlie]

👀 This style of creating the new object to go in our list is how Learn showed working with the student attendance example. However, it turns out we don't need to do this when we send the id of the record we want to report has changed as you did with your onHandleLikeUpdate call. The id passed there can come directly from props, as

    props.onHandleLikeUpdate(props.id);

[anselrognlie]

👀 While making a separate call to update the count of liked messages works, updating the count could also be done in the same function that handles the like count flipping (it can determine whether the number of likes should go up or down). This would help reduce the likelihood of the different pieces of state getting out of sync. But an even more effective mechanism is to eliminate the extra piece of state entirely, and to calculate the number of liked values on render (see additional comments in App.js).

[anselrognlie]

If we eliminate 👀 If we eliminate the local piece of state, we can also eliminate these calls. Note that the setState functions don't return anything, so writing return setHeart('❤️'); is unexpected. First, since the set function returns nothing, this is always explicitly returning undefined. Second, since this is an event handler function (it's handling when the like is clicked), there's not really anywhere for the return value to go, since React will ignore anything the event handler returns.

[anselrognlie]

👍 Nice use of the TimeStamp component.

[anselrognlie]

👀 The number of liked messages can be determined from inspecting the current list of messages. Prefer to avoid introducing additional state when the existing state provides the information we need, and without the risk of state getting out of synch.

We can use a few approaches to get the liked count from the message data. The most javascript-esque way is probably to use reduce, like

  const likeCount = messages.reduce((total, message) => {
    return total + message.liked;  // true will act as 1, false as 0
  }, 0)

Another approach (which makes a copy of the list, but which doesn't really matter since we're mapping this data later anyway) would be

  const likeCount = messages.filter(message => message.liked).length;

Either approach does need to iterate over the message list, which we can avoid with the stand alone count approach. However, since we map over the list later anyway, the additional cost of the iteration here is negligible. If we did end up with other interactions that caused a re-render and didn't want to perform the calculation unnecessarily, there are ways to memoize results in React that can be used to avoid unnecessary calculations. But we should start by deriving values from existing state when possible, and only look for optimizations if the rendering performance is adversely impacted.

[anselrognlie]

👍 The next state of messages depends on the previous state, so using the callback style of the setState method is a good call.

[anselrognlie]

👍 Flipping the liked value here keeps that logic closer to where the state is actually maintained, and with code that needs to "know" what it means to toggle the liked status of a message. Note that we could even move this logic further out of the component so that React doesn't need to know the business rules of these changes. We mostly want to keep React focused on the UI, while flipping the message liked value isn't really a UI concern. React could use a helper to perform this operation without knowing the details of its implementation.

[anselrognlie]

👀 If we go with one of the approaches to calculate the liked count from the message data, then we don't need this function to manage the liked count state.

[anselrognlie]

👀 Removing the liked count state logic would also free us from needing to pass down this additional callback.