finished forgot to commit oh no #103
Conversation
| { | ||
| "python.testing.unittestArgs": [ | ||
| "-v", | ||
| "-s", | ||
| ".", | ||
| "-p", | ||
| "*test.py" | ||
| ], | ||
| "python.testing.pytestEnabled": false, | ||
| "python.testing.unittestEnabled": true | ||
| } |
There was a problem hiding this comment.
Since these are your settings for your own vscode, you don't need to add/commit this file
| @@ -1,3 +1,4 @@ | |||
|
|
|||
There was a problem hiding this comment.
Looks like a blank line was added to the file and then added/committed. we want to avoid bringing in unnecessary changes to a PR. In the future, you can just unstage the change so it doesn't get committed.
| import chatMessages from './data/messages.json'; | ||
| import ChatLog from './components/ChatLog'; | ||
| import { useState } from 'react'; | ||
| import Chats from './data/messages.json'; |
There was a problem hiding this comment.
Since Chats is a constant variable, you can name it with all cap letters like CHATS or you could just call it chats, but it's unusual to see a variable name begin with a capital letter.
|
|
||
| const App = () => { | ||
| const [chatMessages, setChatMessages] = useState(Chats); |
There was a problem hiding this comment.
👍 nice job setting up state for chat messages.
|
|
||
| const App = () => { | ||
| const [chatMessages, setChatMessages] = useState(Chats); | ||
|
|
||
| const heartClick = (entryToUpdate) => { |
There was a problem hiding this comment.
Rather than receiving a new entry with it's like status already changed here, rewrite this method to accept only an id. The logic about how to copy the message and change the liked status would be better if it was written in App.js, since the state must have all the information necessary to make a new message.
const heartClick = id => {
const entries = chatMessageData.map(message => {
if (message.id === id){
return {...message, liked: !message.liked};
}
else {
return message;
}
});
setChatMessages(entries);
}| const ChatEntry = ({id, sender, body, timeStamp, liked, heartClick}) => { | ||
|
|
||
| const clickLikedHeart = () => { | ||
| heartClick( |
There was a problem hiding this comment.
See my comment in app.js about only passing in an id (instead of an entire chat object) into heartClick.
That would make this method look like:
const clickLikedHeart = () => {
heartClick(id);
};| ChatEntry.propTypes = { | ||
| //Fill with correct proptypes | ||
| id: PropTypes.number.isRequired, | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool, | ||
| heartClick: PropTypes.func | ||
| }; |
|
|
||
| const ChatEntry = (props) => { | ||
| return ( | ||
| <div className="chat-entry local"> |
There was a problem hiding this comment.
While it is an optional enhancement to make the local and remote messages display on different sides of the screen, I did want to plant the seed of how you could do that.
If you look in ChatEntry.css, there are 2 classes called 'local' and 'remote'. Right now, on line 18 we've hardcoded one of the class names to be 'local'. But what if we checked to see if the sender === 'Vladimir' and assign a variable to be 'remote' and if it wasn't Vladimir, then the variable would be 'local'.
const senderClass = sender === 'Vladimir' ? 'remote' : 'local';
Then you could use senderClass on line 18 to set the class depending on the sender, like:
<div className={`chat-entry ${senderClass}`}>
| const getChatLog = () => { | ||
| return entries.map((entry) => { | ||
| return ( | ||
| <ChatEntry | ||
| key= {entry.id} | ||
| id= {entry.id} | ||
| sender= {entry.sender} | ||
| body= {entry.body} | ||
| timeStamp={entry.timeStamp} | ||
| liked={entry.liked} | ||
| heartClick= {heartClick} | ||
| /> | ||
| ); | ||
| }); | ||
| }; | ||
| return ( | ||
| <div> | ||
| {getChatLog(entries)} | ||
| </div> |
There was a problem hiding this comment.
Also, instead of writing a method getChatLogs to reference the return value from calling map() on entries, it's more common to directly iterate over the chat entries in your component like this without using a variable:
const ChatLog = ({entries, heartClick}) => {
return (
<div>
{entries.map((entry) => (
<ChatEntry
key={entry.id}
id={entry.id}
sender={entry.sender}
body={entry.body}
timeStamp={entry.timeStamp}
liked={entry.liked}
heartClick={heartClick}
/>
))}
</div>
)
};| key= {entry.id} | ||
| id= {entry.id} | ||
| sender= {entry.sender} | ||
| body= {entry.body} | ||
| timeStamp={entry.timeStamp} | ||
| liked={entry.liked} | ||
| heartClick= {heartClick} |
There was a problem hiding this comment.
Inconsistent whitespaces after = needs to be tidied up before opening a PR
| liked: PropTypes.bool, | ||
| heartClick: PropTypes.func |
There was a problem hiding this comment.
missing .isRequired for these last 2 PropTypes
There was a problem hiding this comment.
oh wait! i remember hearing in the wrap-up session that tests were failing unless you removed .isRequired. Disregard this comment
No description provided.