Skip to content

Sphinx C22 Izzy#23

Open
ihirad wants to merge 12 commits intoAda-C22:mainfrom
ihirad:main
Open

Sphinx C22 Izzy#23
ihirad wants to merge 12 commits intoAda-C22:mainfrom
ihirad:main

Conversation

@ihirad
Copy link

@ihirad ihirad commented Dec 16, 2024

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 +40 to +48
const calculateLikes = (chatData) => {
let total = 0;
for (const chat of chatData) {
if (chat.liked) {
total += chat.liked;
}
}
return total;
};

Choose a reason for hiding this comment

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

You could also do this with the reduce method like so:

const totalLikes = chatMessages.reduce((count, message) => count + (message.liked ? 1 : 0), 0);

Comment on lines +28 to +38
const handleLike = (id) => {
setChatData((chatData) =>
chatData.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.

Nice work on this function! By passing this down to your individual chat messages you are now able to have a single source of truth!

Comment on lines +11 to +24
const senderList = [];
const sendersList = (chatData) => {
for (const chat of chatData) {
if (!senderList.includes(chat.sender)) {
senderList.push(chat.sender);
}
}
let title = '';
for (const sender of senderList) {
title += `${sender} and `;
}
return title.slice(0, -5);
};

Choose a reason for hiding this comment

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

Oh, so this function can include names from 2+ users! Nice!

Comment on lines +56 to +72
const Colors = () => {
const divs = [];
for (let i = 0; i < colors.length; i++) {
const divClass = `${colors[i]} test`;
divs.push(
<div
key={i}
className={divClass}
style={{
backgroundColor: colors[i],
}}
onClick={() => onFontColor(colors[i])}
></div>
);
}
return <div className='color-container'>{divs}</div>;
};

Choose a reason for hiding this comment

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

All the colors of the rainbow! 🌈

Comment on lines +87 to +92
<ChatLog
entries={chatData}
onLike={handleLike}
localSender={local}
fontColor={fontColor}
></ChatLog>

Choose a reason for hiding this comment

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

Parents passing on things to their children, we love it!

Comment on lines +7 to +17
<ChatEntry
key={chat.id}
id={chat.id}
sender={chat.sender}
body={chat.body}
timeStamp={chat.timeStamp}
liked={chat.liked}
onLike={onLike}
localSender={localSender}
fontColor={fontColor}
></ChatEntry>

Choose a reason for hiding this comment

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

Since the names of the keys on entry are the same as the names your are setting on ChatEntry attributes/you are using all of the values in entry you could do something something like this to save you a few keystrokes:

  const chatComponents = entries.map((entry) => {
    return(
      <ChatEntry
        {...entry}
        onLikeToggle={onLikeBtnToggle}
        key={entry.id}
      />
    );
  });

Just recognize that it is not as explicit as what you have. Something to consider!

Comment on lines +25 to +36
entries: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string.isRequired,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
})
).isRequired,
onLike: PropTypes.func,
localSender: PropTypes.string,
fontColor: PropTypes.string,

Choose a reason for hiding this comment

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

"timeStamp":"2018-05-29T22:49:06+00:00",
"liked": false
"liked": false,
"likedCount": 0

Choose a reason for hiding this comment

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

Seems like you decided not use these since we already know that they are liked.

Comment on lines +6 to +8
const onClickLike = () => {
props.onLike(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 +14
const senderPlacement =
props.localSender == props.sender
? 'chat-entry local'
: 'chat-entry remote';

Choose a reason for hiding this comment

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

⭐️

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