-
Notifications
You must be signed in to change notification settings - Fork 111
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
Otters | Tori Shade #92
base: main
Are you sure you want to change the base?
Conversation
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.
Great work, Tori! ✨
Love the additional CSS touches here! 💯 Great job going above and beyond the required features! Your tests pass, your prop and state variables look solid, and your JS looks squeaky clean! This is a well-deserved Green. 🟢
Keep up the great work!
@@ -1,16 +1,46 @@ | |||
import React from 'react'; | |||
import './App.css'; | |||
import chatMessages from './data/messages.json'; | |||
import messages from './data/messages.json'; | |||
import { useState } from 'react'; |
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.
We can combine L1 and L4 into one line because they're both imported from the react library:
import React, { useState } from 'react';
}; | ||
|
||
ChatLog.propTypes = { | ||
entries: PropTypes.array.isRequired, |
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.
We should also add likeCallback
as a required prop type.
|
||
const ChatEntry = (props) => { | ||
|
||
const chatEntryClassType = props.sender === 'Vladimir' ? 'local' : 'remote'; |
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.
Wonderful implementation of the ternary here!
entries: PropTypes.array.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.
Nit: JS wants a new line at the end of the file!
@@ -1,22 +1,37 @@ | |||
import React from 'react'; | |||
import './ChatEntry.css'; | |||
import PropTypes from 'prop-types'; | |||
import TimeStamp from './TimeStamp'; |
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.
Great job using the provided TimeStamp
component ✨
No description provided.