Skip to content
35 changes: 30 additions & 5 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,41 @@
import React from 'react';
import './App.css';
import ChatLog from './components/ChatLog';
import chatMessages from './data/messages.json';
import { useState } from 'react';

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

Choose a reason for hiding this comment

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

👍 We need a piece of state to track changes to our messages, initialized from the json file we loaded.


const updateChatData = (id) => {
setChatData((chatData) =>
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 use of the callback style of setter, since our next state for the chat data depends on the previous state of the chat data.

chatData.map((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.

Nice use of map to make the copy of our array so that React will see that the state has changed.

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.

👍 Nice way to copy our modified message while also toggling the like, without modifying the previous state. We should always make a copy of any data being modified when working with React state.

} else {
return chat;
}
})
);
};

const findTotalHearts = (chatData) => {
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 helper function to tally up the liked message count from state we were already tracking, without the need for an additional piece of state. We could use reduce to handle the looping part of this. In JS, reduce often fills in for operations like sum, min, or max.

let total = 0;
for (const element of chatData) {
if (element.liked === true) {
total += 1;
}
}
return total;
};

const allHearts = findTotalHearts(chatData);

return (
<div id="App">
<header>
<h1>Application title</h1>
</header>
<header>{allHearts} ❤️s</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog entries={chatData} onUpdateChat={updateChatData}></ChatLog>
</main>
</div>
);
Expand Down
19 changes: 14 additions & 5 deletions src/components/ChatEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,29 @@ import './ChatEntry.css';
import PropTypes from 'prop-types';

const ChatEntry = (props) => {
const heartType = 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.

Nice job using the liked value coming from props to update the component display. There's no need to track this with local state, since any time we update the state (using the supplied callback) the component data will update, which we will receive through updated props.


return (
<div className="chat-entry 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.

The optional alignment feature would require us to be able to pick whether to use the local or remote class here. If the App made note of the sender of the first message, and passed that down as a prop, such as localSender, then we could use that value and the value of the current message's sender to determine whether this was a local or remote message.

<h2 className="entry-name">Replace with name of sender</h2>
<h2 className="entry-name">{props.sender}</h2>
<section className="entry-bubble">
<p>Replace with body of ChatEntry</p>
<p className="entry-time">Replace with TimeStamp component</p>
<button className="like">🤍</button>
<p>{props.body}</p>
<p className="entry-time">{props.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.

We should passed in the timeStamp prop value to the time attribute of the supplied TimeStamp component in order to let it take care of converting the time data into something more human-readable.

<button onClick={() => props.onUpdateChat(props.id)} className="like">
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 use of your passed-in callback to allow parent-owned logic to run when the like button is clicked.

{heartType}
</button>
</section>
</div>
);
};

ChatEntry.propTypes = {
//Fill with correct proptypes
id: PropTypes.number.isRequired,
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 prop types. Ideally, we would probably mark all of these props as required, but since there was some test weirdness with the props due to how the test data was provided, it's fine that they're not.

In general, if a property isn't mark required, we should also ensure that our component works properly in its absence. For example, if the callback isn't supplied, we might avoid calling it, or even avoid rendering the heart as clickable at all. Or if the liked value isn't supplied, we might render the message without the liked indicator at all.

sender: PropTypes.string.isRequired,
body: PropTypes.string.isRequired,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool,
onUpdateChat: PropTypes.func,
};

export default ChatEntry;
1 change: 1 addition & 0 deletions src/components/ChatLog.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.chat-log {
margin: auto;
max-width: 50rem;
/* background-color: [orchid]; */
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To set the background color, this would be

  background-color: orchid;

The string color name is bare, without any [] wrapping it.

}
39 changes: 39 additions & 0 deletions src/components/ChatLog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';
import PropTypes from 'prop-types';
import ChatEntry from './ChatEntry';
import './ChatLog.css';

const ChatLog = (props) => {
return (
<div className="chat-log">
<ul>
{props.entries.map((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.

👍 Nice use of map to convert from our array of message data to an array of ChatEntry components.

<ChatEntry
id={chat.id}
sender={chat.sender}
body={chat.body}
timeStamp={chat.timeStamp}
liked={chat.liked}
onUpdateChat={props.onUpdateChat}
key={chat.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.

👍 Good use of the message id as the component key.

/>
))}
</ul>
</div>
);
};

ChatLog.propTypes = {
chats: PropTypes.arrayOf(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍 The prop types look good. As with ChatEntry, these would probably all be required, but we could have left some of them not-required due to the tests.

PropTypes.shape({
sender: PropTypes.string.isRequired,
id: PropTypes.number.isRequired,
body: PropTypes.string.isRequired,
timeStamp: PropTypes.string.isRequired,
liked: 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.

Add .isRequired here to mark the entries prop itself as required.

onUpdateChat: PropTypes.func.isRequired,
};

export default ChatLog;