-
Notifications
You must be signed in to change notification settings - Fork 44
Ada C22 Phoenix Class Madina Dzhetegenova #31
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,14 +1,30 @@ | ||
| import './App.css'; | ||
| import ChatLog from './components/ChatLog'; | ||
| import chatData from './data/messages.json'; | ||
| import { useState } from 'react'; | ||
|
|
||
| const App = () => { | ||
| const [entries, setEntries] = useState(chatData); | ||
| const toggleLike = (id) => { | ||
| const updatedEntries = entries.map((entry) => { | ||
|
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 map here to both handle making a new list so that React sees the message data has changed, and make new data for the clicked message with its like status toggled. |
||
| if (entry.id === id) { | ||
| return { ...entry, liked: !entry.liked }; | ||
| } else { | ||
| return entry; | ||
| } | ||
| }); | ||
| setEntries(updatedEntries); | ||
|
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. In this case, calculating the next version of the message data using the current state variable and passing that updated version to the state setter shouldn't cause any problems, but we still generally prefer using the callback style of setters. Using that approach, we pass a function to the setter whose parameter will receive the latest state value, and which returns the new value to use for the state. setEntries(entries => entries.map(entry => {
if (entry.id === id) {
return {...entry, liked: !entry.liked};
} else {
return entry;
};
})); |
||
| }; | ||
| const likeCount = entries.reduce((total, entry) => { | ||
| return entry.liked ? total + 1 : total; | ||
| }, 0); | ||
|
Comment on lines
+18
to
+20
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 job determining the total likes based on the like data of each message. We don't need an additional piece of state to track this, since it can be derived from the existing state we are tracking. One way that we can make using const sumLikes = (total, entry) => {
return entry.liked ? total + 1 : total;
};
const likeCount = entries.reduce(sumLikes, 0); |
||
| return ( | ||
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>{likeCount} ❤️s</h1> | ||
|
Comment on lines
-7
to
+24
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. 👀 Notice in the example screenshots that in addition to the like counts, there was a header listing the conversation participants that we wanted to show, even if it weren't calculated dynamically. |
||
| </header> | ||
| <main> | ||
| {/* Wave 01: Render one ChatEntry component | ||
| Wave 02: Render ChatLog component */} | ||
| <ChatLog entries={entries} onLikeToggle={toggleLike} /> | ||
| </main> | ||
| </div> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,32 @@ | ||
| import './ChatEntry.css'; | ||
| import TimeStamp from './TimeStamp'; | ||
| import PropTypes from 'prop-types'; | ||
| // import { useState } from 'react'; | ||
|
|
||
| const ChatEntry = () => { | ||
| const ChatEntry = (props) => { | ||
|
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.
|
||
| const heart = 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. 👍 We can figure out which emoji to use for the liked status based on the |
||
| const handleLikeClick = () => { | ||
| props.onLikeToggle(props.id); | ||
|
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. 👍 Passing the |
||
| }; | ||
| return ( | ||
| <div className="chat-entry local"> | ||
| <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"><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 supplied |
||
| <button className="like" onClick={handleLikeClick}>{heart}</button> | ||
|
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. 👍 We need a wrapper of some kind rather than calling the received callback through props, since our callback function is expecting a message id as its parameter. If we tried to use it directly as the click event handler, React would end up passing it a clink event, since any function registered as an event handler will always be given the event detail information as its argument. |
||
| </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, | ||
| onLikeToggle: PropTypes.func.isRequired, | ||
|
Comment on lines
+24
to
+29
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. 👀 The The remaining props were up to you, and the tests don't know about them. As a result, using To properly mark any other props Alternatively, for any props that we leave not required, we should also have logic in our component to not try to use the value if it's undefined. |
||
| }; | ||
|
|
||
| export default ChatEntry; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import ChatEntry from './ChatEntry'; | ||
| import './ChatLog.css'; | ||
|
|
||
| const ChatLog = ({entries, onLikeToggle}) => { | ||
| const chatComponents = entries.map((entry) => { | ||
|
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 |
||
| return ( | ||
| <ChatEntry | ||
| key={entry.id} | ||
| id={entry.id} | ||
|
Comment on lines
+9
to
+10
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. 👍 The |
||
| sender={entry.sender} | ||
| body={entry.body} | ||
| timeStamp={entry.timeStamp} | ||
| liked={entry.liked} | ||
| onLikeToggle={onLikeToggle} | ||
| /> | ||
| ); | ||
| }); | ||
|
|
||
| return <div className="chat-log">{chatComponents}</div>; | ||
| }; | ||
|
|
||
| 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, | ||
|
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. We can make the |
||
| }) | ||
| ).isRequired, | ||
| onLikeToggle: PropTypes.func.isRequired | ||
|
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. Similar to the props for Again, if we were to leave this as not required so as to avoid the test warnings, we'd want to be sure that all the script logic in our component worked properly even in the absence of this value. |
||
| }; | ||
|
|
||
| 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.
👍 Since our state is defined here, we also need to define our mutating function here so that it can "see" the setter function. All we need to receive is the
idof the message to toggle, which allows us to locate the message to update, and calculate the next state value. We can pass this all the way down to ourChatEntrywhich handles the click event, passing us theidof the message that was clicked.