Skip to content

Conversation

@Leidy-Martinez
Copy link

No description provided.

Copy link

@mikellewade mikellewade left a comment

Choose a reason for hiding this comment

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

Nice work! Your project lets me know that you have a goo understanding of React and useState!

Comment on lines +7 to +11
const calculateTotalLikeCount = (chat) => {
return chat.reduce((total, chat) => {
return total + (chat.liked ? 1 : 0);
}, 0);
};

Choose a reason for hiding this comment

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

Excellent execution!

Comment on lines +13 to +21
const localSender = messages[0].sender;
let remoteSender = 'Unknown';

for (const message of messages) {
if (message.sender !== localSender) {
remoteSender = message.sender;
break; // Stop once other sender is found
}
}

Choose a reason for hiding this comment

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

Nice this handles cases of double texting!

Comment on lines +27 to +37
const handleLikeButtom = (id) => {
setChat(chat => {
return chat.map(chat => {
if (chat.id === id) {
return { ...chat, liked: !chat.liked };
} else {
return chat;
}
});
});
};

Choose a reason for hiding this comment

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

No notes on this! Shows me that you have a great understanding of lifting up state!



const App =() => {
const [chat, setChat] = useState(messages);

Choose a reason for hiding this comment

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

Maybe we could change this from chat to chats since it is multiple chat messages instead of one?

Comment on lines +49 to +53
<ChatLog
entries={chat}
onLiked={handleLikeButtom}
localSender={localSender}
likes={totalLikes} />}

Choose a reason for hiding this comment

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

Does totalLikes actually need to be passed? We are only using it in line 44.

const ChatLog = (props) => {
const chatentrycomponents = props.entries.map((entries, index) => {
return (
<ul key={index}>

Choose a reason for hiding this comment

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

Since we have the ids of entries we could actually use that. We want to avoid using the indexes.



const ChatLog = (props) => {
const chatentrycomponents = props.entries.map((entries, index) => {

Choose a reason for hiding this comment

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

We could maybe name this entry instead of entries since it only a single entry we will be looking at each iteration.

Comment on lines +32 to +43
entries: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
})
).isRequired,

onLiked: PropTypes.func.isRequired,
localSender: PropTypes.string.isRequired,

Choose a reason for hiding this comment

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

⭐️

Choose a reason for hiding this comment

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

I see that the tests are saying that some of the props are undefined but that isn't the case. I'm not entirely sure why.

Comment on lines +7 to +9
const likeButtonCliked = () => {
props.onLiked(props.id);
};

Choose a reason for hiding this comment

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

The button function could also be implemented like so:

<button className='like' onClick={() => likeButtonClicked(id)}>{liked ? '❤️': '🤍'}</button>

Comment on lines +11 to +12
const heartColor = props.liked ? '❤️' : '🤍';
const messageClass = props.sender === props.localSender ? 'local' : 'remote';

Choose a reason for hiding this comment

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

Ternaries, love to see them!

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