-
Notifications
You must be signed in to change notification settings - Fork 113
D19 Kunzite - Abby Castillo #108
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
212d3ac
217c191
fadad84
89c1aa3
b887bb1
46f609d
9b4c2ef
0fdf48b
e7fb86c
be6c5b1
1016d29
d7eda22
23a80ee
c4dcbf1
39c7a8f
3f7c571
81b070a
e6cc60e
4181066
fac0d81
655a19b
94e3ca9
e0a534b
138a33a
dadeef6
59e80b7
0bf2089
7815aae
a4c6e57
fe3de7c
b2ed277
77ac454
80c29f2
7303bbb
968a7bc
86ed929
1a143c2
dc42f7e
8bc5806
11375f6
9c5a468
1ea8554
03e0096
12511c9
4d7a8e4
ecf20b6
6affa1f
ab27f99
99a4d52
05eb8c5
fc817b4
72534b7
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 |
|---|---|---|
|
|
@@ -71,4 +71,8 @@ | |
|
|
||
| .purple { | ||
| color: purple | ||
| } | ||
| } | ||
|
|
||
| /* button like { | ||
|
|
||
| } */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,48 @@ | ||
| import React from 'react'; | ||
| import React, {useState} from 'react'; | ||
| import './ChatEntry.css'; | ||
| import PropTypes from 'prop-types'; | ||
| import TimeStamp from './TimeStamp'; | ||
| import LikeButton from './LikeButton'; | ||
|
|
||
| const setMessageLocation = (userName) => { | ||
|
|
||
| if (userName === 'Vladimir') { | ||
| return 'chat-entry local'; | ||
| } else if (userName === 'Estragon') { | ||
| return 'chat-entry remote'; | ||
| }; | ||
|
|
||
| } | ||
|
|
||
| const ChatEntry = ({id, sender, body, timeStamp, isLiked, onLikeMessage}) => { | ||
|
|
||
| const updateLike = () => { | ||
| onLikeMessage(id); | ||
| }; | ||
|
|
||
| const senderLocation = setMessageLocation(sender); | ||
|
|
||
| const ChatEntry = (props) => { | ||
| return ( | ||
| <div className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <div className={senderLocation}> | ||
| <h2 className="entry-name">{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>{body}</p> | ||
| <p className="entry-time"><TimeStamp time={timeStamp}/></p> | ||
|
|
||
| <LikeButton heartCondition={isLiked} updateLike={updateLike}/> | ||
| </section> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| ChatEntry.propTypes = { | ||
| //Fill with correct proptypes | ||
| id: PropTypes.number.isRequired, | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| isLiked: PropTypes.bool.isRequired, | ||
| onLikeMessage: PropTypes.func.isRequired, | ||
| }; | ||
|
|
||
| export default ChatEntry; | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import React from 'react'; | ||
| import ChatEntry from './ChatEntry'; | ||
| import PropTypes from 'prop-types'; | ||
| import './ChatLog.css'; | ||
|
|
||
| const ChatLog = ({entries, onLikeMessage}) => { | ||
| const chatComponents = entries.map((message) => { | ||
| return ( | ||
| <div key={message.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. Typically, we'd wrap a component in additional JSX if there was a structural or style consideration. Not sure I see either factoring in here, so you could get rid of the additional JSX and just return the ChatEntry component. In general, you'd prefer not adding unnecessary markup. |
||
| <ChatEntry | ||
| id={message.id} | ||
| sender={message.sender} | ||
| body={message.body} | ||
| timeStamp={message.timeStamp} | ||
| isLiked={message.liked} | ||
| onLikeMessage={onLikeMessage} | ||
| /> | ||
| </div> | ||
| ); | ||
| }); | ||
|
|
||
| return (<section className='chat-log'> | ||
| {chatComponents} | ||
| </section>) | ||
| }; | ||
|
|
||
| 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 | ||
| })), | ||
| onLikeMessage: PropTypes.func.isRequired, | ||
| }; | ||
|
|
||
| export default ChatLog; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import '../App.css'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const HeartCounter = (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. I like how you decided to create additional components that are controlled by props. It encourages reusability, which is an important thing in React. |
||
|
|
||
| return ( | ||
| <div> | ||
| <p className='heartWidget'> | ||
| {props.likeTotal} ❤️s | ||
| </p> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| HeartCounter.propTypes = { | ||
| likeTotal: PropTypes.number.isRequired, | ||
| }; | ||
|
|
||
| export default HeartCounter; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const LikeButton = (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. Again, I like how you created a new component to abstract the like button and control it with props 👍🏽 |
||
| if (props.heartCondition) { | ||
|
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 kind of conditional works fine but is a bit repetitive. It would work well to set the heart emoji based on a condition in a ternary and thereby reduce the amount of JSX: const LikeButton = (props) => (
<button className="like" onClick={props.updateLike}>
{props.heartCondition ? '❤️' : '🤍'}
</button>
); |
||
| return <button onClick={props.updateLike}>❤️</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. Some tests are failing because there is a class that the tests are using in order to be selected and fire click events on (see line 24 in App.test.js). If you add the |
||
| } else { | ||
| return <button className='like' onClick={props.updateLike}>🤍</button> | ||
| }; | ||
| }; | ||
|
|
||
| LikeButton.propTypes = { | ||
| heartCondition: PropTypes.bool.isRequired, | ||
| updateLike: PropTypes.func.isRequired | ||
| } | ||
|
|
||
| export default LikeButton; | ||
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.
Here, you're mutating the original message data that's imported on line 5 above. This was actually causing some tests to fail, in conjunction with a few other points mentioned above and below. For React, you'll want to be cautious about such mutations as it can cause unexpected bugs. The best approach to updating data is actually to create new objects, e.g.
But better still would be to do this while also leveraging the approach outlined above of reading the previous data passed to a callback function of the state updating function, e.g.
and then calculating the heart total separately