Skip to content

Mengqiao Han Chatlog#26

Open
Bipentium25 wants to merge 6 commits intoAda-C24:mainfrom
Bipentium25:main
Open

Mengqiao Han Chatlog#26
Bipentium25 wants to merge 6 commits intoAda-C24:mainfrom
Bipentium25:main

Conversation

@Bipentium25
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@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.

@Bipentium25, overall great work! I will say that consistent area of growth that I have seen for you across all projects is your formatting. It is usually inconsistent across your codebase. You will want to pay close attention to convention and style guides when you implement work in internship.

Comment thread src/App.jsx
setChats(chatData =>
chatData.map(chat =>{
if (chat.id === id) {
return{ ...chat, liked: !chat.liked };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return{ ...chat, liked: !chat.liked };
return { ...chat, liked: !chat.liked };

Comment thread src/App.jsx
Comment on lines +23 to +27
const totalLikes = () => {
return chatData.reduce((count, chat) => {
return chat.liked ? count + 1 : count;
}, 0);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work on this function implementation, @Bipentium25. Using the .reduce method is a very JS idiomatic way to code this logic.

Comment thread src/App.jsx
Comment on lines +29 to +30
const local = chatData[0].sender;
const remote = chatData.find(chat => chat.sender !== local).sender;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comment thread src/App.jsx
Comment on lines +35 to +36
<h1>Chat Between {local} and {remote}</h1>
<section><h2 className='widget'>{totalLikes()} ❤️s</h2></section>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍🏿

Comment thread src/App.jsx
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog entries={chatData} toggleLike={toggleLike} local={local}/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice! I see that you leveraged a local prop inside of your Chatlog component to mark all of your entry objects with a matching sender as the local messages.


const ChatEntry = () => {
const ChatEntry = ({ id, sender, body, timeStamp, liked, toggleLike = () => {}, isLocal = false }) => {
const heart = liked ? '❤️' : '🤍';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perfect use case for a ternary operator! This is how we implement conditional rendering.

<p>Replace with body of ChatEntry</p>
<p className="entry-time">Replace with TimeStamp component</p>
<button className="like">🤍</button>
<div className={`chat-entry ${isLocal ? 'local' : 'remote'}`}>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another great use of ternary operators!

<section className='entry-bubble'>
<p>{body}</p>
<p className='entry-time'><TimeStamp time={timeStamp}></TimeStamp></p>
<button className='like' onClick={() => toggleLike(id)}>{heart}</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perfect!

Comment on lines +10 to +13
<h2 className='entry-name'>{sender}</h2>
<section className='entry-bubble'>
<p>{body}</p>
<p className='entry-time'><TimeStamp time={timeStamp}></TimeStamp></p>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👌

Comment on lines +21 to +28
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string.isRequired,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
toggleLike: PropTypes.func.isRequired,
isLocal: PropTypes.bool,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't forget that in React PropTypes have been sunsetted and you will want to use Typescript in practice now!

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