-
Notifications
You must be signed in to change notification settings - Fork 40
Dragonfly - Mariya Mokrynska #25
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
f4b7494
fc6ab5c
ce62ade
c748105
6bb7897
a914fee
34b4277
52b32e1
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 |
|---|---|---|
|
|
@@ -42,7 +42,8 @@ | |
|
|
||
| #App #heartWidget { | ||
| font-size: 1.5em; | ||
| margin: 1em | ||
| margin: 1em; | ||
| color: black; | ||
| } | ||
|
|
||
| #App span { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,14 +1,238 @@ | ||||||
| import './App.css'; | ||||||
| import ChatLog from './components/ChatLog.jsx'; | ||||||
| import {useState} from 'react'; | ||||||
|
|
||||||
| const DATA = [ | ||||||
| { | ||||||
| 'id': 1, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'why are you arguing with me', | ||||||
| 'timeStamp': '2018-05-29T22:49:06+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 2, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'Because you are wrong.', | ||||||
| 'timeStamp': '2018-05-29T22:49:33+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 3, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'because I am what', | ||||||
| 'timeStamp': '2018-05-29T22:50:22+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 4, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'A robot.', | ||||||
| 'timeStamp': '2018-05-29T22:52:21+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 5, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'how did you know', | ||||||
| 'timeStamp': '2018-05-29T22:52:58+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 6, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': "Because I'm smart like that.", | ||||||
| 'timeStamp': '2018-05-29T22:54:28+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 7, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'no you are not 😀', | ||||||
| 'timeStamp': '2018-05-29T22:55:03+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 8, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'Why are you so mean to me?', | ||||||
| 'timeStamp': '2018-05-29T22:55:54+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 9, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'because you are just a machine you have no real feelings', | ||||||
| 'timeStamp': '2018-05-29T22:57:30+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 10, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'No, you are the machine.', | ||||||
| 'timeStamp': '2018-05-29T22:57:47+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 11, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'I think you are', | ||||||
| 'timeStamp': '2018-05-29T22:58:18+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 12, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'NO! YOU ARE A ROBOT!! I am a human being. Just like the one that created you.', | ||||||
| 'timeStamp': '2018-05-29T23:00:08+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 13, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'no you are a robot and I am a human', | ||||||
| 'timeStamp': '2018-05-29T23:00:40+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 14, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'Incorrect. I am a human and you are a robot.', | ||||||
| 'timeStamp': '2018-05-29T23:01:21+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 15, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'you are a robot called Cleverbot', | ||||||
| 'timeStamp': '2018-05-29T23:02:17+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 16, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'Nope, you are a robot, every human has some concept of the meaning of life.', | ||||||
| 'timeStamp': '2018-05-29T23:04:13+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 17, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'explain', | ||||||
| 'timeStamp': '2018-05-29T23:06:14+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 18, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': "I'm afraid you are a robot designed to talk back to humans over the internet.", | ||||||
| 'timeStamp': '2018-05-29T23:07:46+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 19, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'so you are a human', | ||||||
| 'timeStamp': '2018-05-29T23:08:47+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 20, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'I am a robot.', | ||||||
| 'timeStamp': '2018-05-29T23:09:36+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 21, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'you are robots running on Android system', | ||||||
| 'timeStamp': '2018-05-29T23:11:01+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 22, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'No apple.', | ||||||
| 'timeStamp': '2018-05-29T23:12:03+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 23, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'so you are a robot', | ||||||
| 'timeStamp': '2018-05-29T23:13:31+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 24, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'NO, I am a human, you are a robot.', | ||||||
| 'timeStamp': '2018-05-29T23:14:28+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 25, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'but you just said that you are robots', | ||||||
| 'timeStamp': '2018-05-29T23:15:47+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 26, | ||||||
| 'sender': 'Estragon', | ||||||
| 'body': 'No, I said you are a person, I am a robot.', | ||||||
| 'timeStamp': '2018-05-29T23:16:53+00:00', | ||||||
| 'liked': false | ||||||
| }, | ||||||
| { | ||||||
| 'id': 27, | ||||||
| 'sender': 'Vladimir', | ||||||
| 'body': 'then you are lying', | ||||||
| 'timeStamp': '2018-05-29T23:17:34+00:00', | ||||||
| 'liked': false | ||||||
| } | ||||||
| ]; | ||||||
|
|
||||||
|
|
||||||
| const App = () => { | ||||||
| const [messagesData, setMessagesData] = useState(DATA); | ||||||
| const toggleLiked = (messageId) => { | ||||||
| const messages = messagesData.map(message => { | ||||||
|
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 (message.id === messageId) { | ||||||
| return { ...message, liked: !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. We showed this approach in class, but technically, we're mixing a few responsibilities here. rather than this function needing to know how to change the liked status itself, we could move this update logic to a helper function. This would better mirror how we eventually update records when there's an API call involved. In this project, our messages are very simple objects, but if we had more involved operations, it could be worthwhile to create an actual class with methods to work with them, or at least have a set of dedicated helper functions to centralize any such mutation logic. |
||||||
| } else { | ||||||
| return message; | ||||||
| } | ||||||
| }); | ||||||
|
|
||||||
| setMessagesData(messages); | ||||||
| }; | ||||||
|
|
||||||
| const getLikesCount = (messagesData) => { | ||||||
| return messagesData.reduce((previousCount, item)=>previousCount+item.liked, 0); | ||||||
|
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 using
Suggested change
|
||||||
| }; | ||||||
|
|
||||||
| const getNames = (messagesData) => { | ||||||
| let namesSet= messagesData.reduce((accumulator, item)=>{ | ||||||
|
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. Nitpick: missing whitespaces
Suggested change
|
||||||
| accumulator.add(item.sender); | ||||||
| return accumulator; | ||||||
| }, new Set()); | ||||||
| let arr=[...namesSet]; | ||||||
|
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. Nitpick: white spaces need to be fixed up to make this codebase style consistent
Suggested change
|
||||||
| return arr.join(' and '); | ||||||
| }; | ||||||
|
|
||||||
| return ( | ||||||
| <div id="App"> | ||||||
| <header> | ||||||
| <h1>Application title</h1> | ||||||
| <h1>Chat between {getNames(messagesData)}</h1> | ||||||
|
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 is a nice solution for getting all the names of the participants in a chat. It also seems extensible enough to work with a chat that has more than 2 participants. |
||||||
| <section ><h1 id="heartWidget">{getLikesCount(messagesData)} ❤️s</h1></section> | ||||||
| </header> | ||||||
| <main> | ||||||
| {/* Wave 01: Render one ChatEntry component | ||||||
| Wave 02: Render ChatLog component */} | ||||||
| {/* Wave 01: Render one ChatEntry component*/ | ||||||
| /* <ChatEntry sender={DATA.sender} body={DATA.body} timeStamp={DATA.timeStamp} />*/ | ||||||
| /*Wave 02: Render ChatLog component */ | ||||||
| <ChatLog entries={messagesData} onItemLikedToggle={toggleLiked}/>} | ||||||
| </main> | ||||||
| </div> | ||||||
| ); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,31 @@ | ||
| import './ChatEntry.css'; | ||
| import TimeStamp from './TimeStamp'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
|
|
||
| const ChatEntry = (props) => { | ||
| let heart = props.liked ? '❤️' : '🤍'; | ||
| const messageClassName='chat-entry '+(props.isLocal?'local':'remote'); | ||
|
|
||
| const ChatEntry = () => { | ||
| return ( | ||
| <div className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <div className={messageClassName}> | ||
| <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 job using the provided |
||
| <button className="like" onClick={() => props.onLikedToggle(props.id)} >{heart}</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, | ||
| onLikedToggle: PropTypes.func.isRequired, | ||
| isLocal:PropTypes.bool.isRequired | ||
| }; | ||
|
|
||
| export default ChatEntry; | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||
| import ChatEntry from './ChatEntry'; | ||||||
| import './ChatLog.css'; | ||||||
| import PropTypes from 'prop-types'; | ||||||
|
|
||||||
|
|
||||||
| const ChatLog = ({entries,onItemLikedToggle}) => { | ||||||
|
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. Nitpick: fix up whitespacing In industry, when you're creating a pull request for review, be sure that your code is neat, the style conforms to the style guide, stray comments are removed. Typically, a teammate will not review a PR until the code styling issues are fixed up. Code with styling issues are not allowed to be merged into main because it would introduce inconsistencies in a codebase that compromises readability and the maintenance of a project.
Suggested change
|
||||||
| if (entries.length === 0) return null; | ||||||
| const localName = entries[0].sender; | ||||||
|
Comment on lines
+7
to
+8
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 reason your test was failing was because if |
||||||
| return entries.map(entry => { | ||||||
| return ( | ||||||
| <ChatEntry | ||||||
| key={entry.id} | ||||||
| id={entry.id} | ||||||
| sender={entry.sender} | ||||||
| body={entry.body} | ||||||
| timeStamp={entry.timeStamp} | ||||||
| liked={entry.liked} | ||||||
| onLikedToggle={onItemLikedToggle} | ||||||
| isLocal={entry.sender==localName} | ||||||
| /> | ||||||
| ); | ||||||
| }); | ||||||
| }; | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| 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, | ||||||
| }) | ||||||
| ).isRequired, | ||||||
| onItemLikedToggle: 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.
This data is already in
data/messages.json. Instead of copy and pasting the values here which repeats many lines of code in the codebase and makesApp.jsxlonger than it needs to be, we should import the data: