PR for react-chatlog instructor review

[yangashley]

No description provided.

[yangashley]

FYI @SalmaAnany the link you submitted in Learn wasn't to a valid PR, so I opened this so I can add review comments to it.

[yangashley]

Let me know if you have any questions about my review comments.

[yangashley]

App.jsx should have the following (you can reference examples of how we did this in flasky-front-end, review this link here):

• function to calculate the number of likes from each ChatEntry (see lines 83-87 and lines 89, 95 in App.jsx in the link above)
• state that represents all the messages (see line 42 in App.jsx in the link above)
• an event handling function that updates the state when a message is liked or unliked so the page can re-render that is passed down as a prop to ChatLog.jsx and then down to ChatEntry.jsx (see lines 44-55 and line 98 in App.jsx in the link above)

[yangashley]

We don't need to track if a ChatEntry has been liked with state here.

If App.jsx knows about all the components (which is why we lift state out of ChatEntry and up to App), then we can get rid of state here.

See how the Cat.jsx component is structured here. We can think of Cat as being equivalent to ChatEntry.

[yangashley]

Instead of onClick being an anonymous function with logic, I'd expect the onClick event handler to invoke a function that is passed down from App.jsx to ChatLog.jsx and finally down to this component here ChatEntry.jsx.

You can find an example of what this looks like at this link, see lines 17-21 and line 35 in Cat.jsx

Petting a cat in flasky-front-end is the equivalent of liking a ChatEntry.

[yangashley]

This line is challenging to read and violates the JS styleguide that suggests each line is no more than 120 characters long.

Using new lines for each prop being passed would help with readability.

[yangashley]

Prefer a more descriptive name than i. Maybe entry?

[yangashley]

This element should live in App.jsx, at the highest level of the hierarchy of all the components.

You can see an example of how this looks in App.jsx on line 95 here in flasky-front-end

[yangashley]

Your refactored project looks great!

[yangashley]

👍

[yangashley]

You could also use reduce too:

  // adding a bool to a number treats true as 1 and 0 as false
  const totalLikes = chatEntries.reduce((totalLikes, entry) => (totalLikes + entry.liked), 0);

[yangashley]

Yep, exactly the props that need to get passed down.

[yangashley]

Typically you'll see the result of using map referenced by a variable and using that variable in the JSX, like:

  const chatLogs =  entries.map((entry) => {
	      return (
	          <ChatEntry                                 
	            key={entry.id}
                    {...entry}
                    onLikeToggle={onLikeToggle}
                    isLocal={entry.sender === 'Vladimir'}
	          />
	        );
	     });
  };

  return (
        <div>
            {chatLogs}
        </div>
  );

[yangashley]

👍