Skip to content

Crow 24 - Tsz Tin Vivian Soo#20

Open
tsztin0217 wants to merge 7 commits intoAda-C24:mainfrom
tsztin0217:main
Open

Crow 24 - Tsz Tin Vivian Soo#20
tsztin0217 wants to merge 7 commits intoAda-C24:mainfrom
tsztin0217:main

Conversation

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

@tsztin0217, great work on your React Project!

Comment on lines +7 to +15
key={entry.id}
id={entry.id}
sender={entry.sender}
body={entry.body}
timeStamp={entry.timeStamp}
liked={entry.liked}
onLikeToggle={props.onLikeToggle}
localColor={props.localColor}
remoteColor={props.remoteColor}
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, especially on the formatting. To simplify your logic, you could pass in the entire message object yourself. However, you would lose out on the explicitness you have here.

Comment thread src/App.jsx
Comment on lines +8 to +15
const handleLikedChat = chat => {
return { ...chat, liked: !chat.liked};
};

const countLikes = chatData => {
return chatData.reduce((sum, chat) => {
return sum + (chat.liked ? 1: 0);
}, 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.

Nice work on these functions, especially with having them defined outside of the component since they aren't dependent on the locally scoped data like state.

Comment thread src/App.jsx


const handleLikedChat = chat => {
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
};

const countLikes = chatData => {
return chatData.reduce((sum, chat) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Though you will find a number of implementations for this, the .reduce method is the one I would prefer for the given case.

Comment thread src/App.jsx
};

const App = () => {
const [chatData, setChatData] = useState(DATA);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

props.onLikeToggle(props.id);
};

const likeButtonColor = props.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.

Great use of a ternary!

import TimeStamp from './TimeStamp';
import PropTypes from 'prop-types';

const ChatEntry = (props) => {
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 you can de-structure your props here! Though this does readily display that you are working with props and not local variables declared in the components.

const ChatEntry = (props) => {

const likeButtonClicked = () => {
props.onLikeToggle(props.id);
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 +16 to +21
<article className={`chat-entry ${isLocal ? 'local' : 'remote'}`}>
<h2 className="entry-name">{props.sender}</h2>
<section className={`entry-bubble ${colorClass}`}>
<p>{props.body}</p>
<p className="entry-time"><TimeStamp time={props.timeStamp}></TimeStamp></p>
<button className="like" onClick={likeButtonClicked}>{likeButtonColor}</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.

Comment on lines +28 to +35
sender: PropTypes.string,
body: PropTypes.string,
timeStamp: PropTypes.string,
liked: PropTypes.bool,
id: PropTypes.number,
onLikeToggle: PropTypes.func,
localColor: PropTypes.string,
remoteColor: PropTypes.string,
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 these proptypes, remember that they have been sunsetted in React and that will be using other type checking frameworks like Typescript.

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