Skip to content

Conversation

@erinaperez
Copy link

No description provided.

Copy link

@marciaga marciaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! A couple minor comments below but overall nice work!

import './App.css';
import chatMessages from './data/messages.json';
import ChatLog from './components/ChatLog.js';
import { useState } from 'react';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we've already got a line that imports React, we want to avoid adding another line that duplicates it, so better to add to line 1:

import React, { useState } from 'react';

return {
...entry,
liked: !entry.liked,
}; } else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this else should be on a new line:

if(id === entry.id) {
  return {
    ...entry,
    liked: !entry.liked,
  };
} else {
...
}      

});
}
const totalLikesTally = entries.reduce((total, entry) => {
if (entry.liked === true) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok to compare entry.liked to a boolean but it would be more common to just check for truthinees:

if (entry.liked) {

});
});
}
const totalLikesTally = entries.reduce((total, entry) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of reduce() here!

{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog
className='chat-log'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing but these props should be indented two spaces to the right

import ChatEntry from './ChatEntry.js';

const ChatLog = (props) => {
const chatComponents = props.entries.map(entry => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learn showed this pattern of creating an array of React elements and then referencing it in a JSX expression, so it's definitely okay to do here, but I should just add that I have not found it to be common pattern in practice - most people would just return the array in the component's return statement, e.g.

return (
  <div>
    {props.entries.map(entry => (
      <ChatEntry
        key={entry.id} 
        id={entry.id}  
        sender={entry.sender}
        body={entry.body}
        liked={entry.liked}
        timeStamp={entry.timeStamp}
        updateLikedCount={props.updateLikedCount}
      />
    )}
  </div>
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants