-
Notifications
You must be signed in to change notification settings - Fork 153
Snow Leopard Jessica H. C18 #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,43 @@ | ||
import React from 'react'; | ||
import { useState } from 'react'; | ||
import './App.css'; | ||
import chatMessages from './data/messages.json'; | ||
import ChatLog from './components/ChatLog.js'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file name is ChatLog.js here, but the name is actually Chatlog.js which prevents the code from compiling. Did you run into any errors when trying to push up the ChatLog.js to your github repo? |
||
|
||
const chatDataList = chatMessages | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need to use another variable to reference chatMessages. On line 4 when you import the data from './data/messages.json' you reference it with chatMessages so from there on out you can just refer to the data with chatMessages. |
||
|
||
const App = () => { | ||
const [chatData, setChatData] = useState(chatDataList); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of setting state with chatDataList, you can set the state with chatMessages directly |
||
|
||
const toggleHeart = (id) => { | ||
setChatData(chatData => chatData.map(message => { | ||
if(message.id === id) { | ||
return {...message, liked: !message.liked} | ||
} else { | ||
return message; | ||
} | ||
})); | ||
} | ||
|
||
const calcHearts = (chatData) => { | ||
return chatData.reduce((total, message) => { | ||
return total + message.liked; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Way to use reduce here! It's cool that you leveraged the knowledge that in JavaScript if you add a boolean to something it gets cast as a number (0 for false, 1 for true). Prefer you use conditional logic to add a number to a number instead, like: return chat.liked ? total + 1 : total; |
||
}, 0) | ||
}; | ||
|
||
const totalHearts = calcHearts(chatData); | ||
|
||
return ( | ||
<div id="App"> | ||
<header> | ||
<h1>Application title</h1> | ||
<h1>My Chat Log</h1> | ||
</header> | ||
<main> | ||
{/* Wave 01: Render one ChatEntry component | ||
Wave 02: Render ChatLog component */} | ||
<h2>{totalHearts} ❤️s</h2> | ||
<ChatLog entries={chatData} onToggleHeart ={toggleHeart}/> | ||
</main> | ||
</div> | ||
); | ||
}; | ||
|
||
export default App; | ||
export default App; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,31 @@ | ||
import React from 'react'; | ||
import './ChatEntry.css'; | ||
import PropTypes from 'prop-types'; | ||
import TimeStamp from './TimeStamp'; | ||
|
||
const ChatEntry = (props) => { | ||
const heartColor = props.liked ? '❤️' : '🤍'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
return ( | ||
<div className="chat-entry local"> | ||
<h2 className="entry-name">Replace with name of 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> | ||
<h2 className="entry-name">{props.sender}</h2> | ||
<section className="entry-bubble"> | ||
<p>{props.body}</p> | ||
<p className="entry-time"><TimeStamp time={props.timeStamp}/></p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of the provided TimeStamp component |
||
<button className="like" onClick={() => props.onToggleHeart(props.id)}>{heartColor}</button> | ||
</section> | ||
</div> | ||
); | ||
}; | ||
|
||
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.isRequired, | ||
onToggleHeart: PropTypes.func.isRequired, | ||
}; | ||
|
||
export default ChatEntry; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import PropTypes from 'prop-types'; | ||
import ChatEntry from './ChatEntry.js'; | ||
|
||
const ChatLog = (props) => { | ||
|
||
return ( | ||
<> | ||
{props.entries.map((entry) => ( | ||
<ChatEntry | ||
id={entry.id} | ||
sender={entry.sender} | ||
body={entry.body} | ||
timeStamp={entry.timeStamp} | ||
key={entry.id} | ||
liked={entry.liked} | ||
onToggleHeart={props.onToggleHeart} | ||
/> | ||
))} | ||
</> | ||
) | ||
|
||
} | ||
|
||
ChatLog.propTypes = { | ||
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, | ||
})), | ||
onToggleHeart: PropTypes.func.isRequired, | ||
} | ||
|
||
export default ChatLog; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at this "Files changed" section of your pull request it says that 1472 files were committed and pushed to the repo. It looks like you created a virtual environment and then pushed up all the files associated with it. React applications do not need a virtual environment (but Flask projects do). Be mindful of what files you push up to a repository and add to a PR.
It can be helpful to use
git add SpecificFileName.js
instead of usinggit add .
so that you don't accidentally add and commit files you don't mean to.In industry, the person reviewing your PR would ask that you resubmit this PR without the venv directory and files so you'd need to figure out how to remove them from your commit history and push the changes again.