-
Notifications
You must be signed in to change notification settings - Fork 35
crow-lexy #15
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?
crow-lexy #15
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,31 @@ | ||
| import './App.css'; | ||
| import ChatLog from './components/ChatLog'; | ||
| import messages from './data/messages.json'; | ||
| import { useState} from 'react'; | ||
|
|
||
| const App = () => { | ||
| const [entries, setEntries] = useState(messages); | ||
|
|
||
| const handleLike = (id) => { | ||
| setEntries((prevEntries) => | ||
| prevEntries.map((entry) => | ||
| entry.id === id ? { ...entry, liked: !entry.liked } : entry | ||
| ) | ||
| ); | ||
| }; | ||
|
Comment on lines
+9
to
+15
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 work ensuring we're creating a new array of messages to trigger the re-render after updating the liked value! |
||
|
|
||
| const totalLikes = entries.reduce((acc, entry) => acc + Number(entry.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. Great use of the existing data and |
||
|
|
||
| return ( | ||
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <section className='widget'> | ||
| <p>{totalLikes} ❤️s</p> | ||
| </section> | ||
| </header> | ||
| <main> | ||
| {/* Wave 01: Render one ChatEntry component | ||
| Wave 02: Render ChatLog component */} | ||
| <ChatLog entries = {entries} onHandleLike={handleLike} /> | ||
| </main> | ||
| </div> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,33 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import './ChatEntry.css'; | ||
| import TimeStamp from './TimeStamp'; | ||
|
|
||
| const ChatEntry = () => { | ||
|
|
||
| const ChatEntry = ({ sender, body, timeStamp, liked, onHandleLike }) => { | ||
| const heart = liked ? '❤️' : '🤍'; | ||
| return ( | ||
| // Replace the outer tag name with a semantic element that fits our use case | ||
| <replace-with-relevant-semantic-element className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <article className="chat-entry local"> | ||
| <h2 className="entry-name">{sender}</h2> | ||
|
Comment on lines
+10
to
+11
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. If we wanted to apply the optional enhancements to align the chat bubbles, we could use a ternary operator to decide the remote or local |
||
| <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> | ||
|
Comment on lines
+14
to
+16
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. Great thought to wrap this in a |
||
| <button className="like" onClick={onHandleLike}> | ||
| {heart} | ||
| </button> | ||
|
Comment on lines
+17
to
+19
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 increase the accessibility of our page by setting further attributes on elements, especially interactive elements. Below is an example of updates we could make to the <button
onClick={onHandleLike}
className="like"
aria-label={heart}
role="img"
>
{heart}
</button> |
||
| </section> | ||
| </replace-with-relevant-semantic-element> | ||
| </article> | ||
| ); | ||
| }; | ||
|
|
||
| ChatEntry.propTypes = { | ||
| // Fill with correct proptypes | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool.isRequired, | ||
| onHandleLike: PropTypes.func, | ||
| }; | ||
|
|
||
| export default ChatEntry; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import './ChatLog.css'; | ||
| import ChatEntry from './ChatEntry'; | ||
|
|
||
|
|
||
| const ChatLog = ({ entries, onHandleLike }) => { | ||
| const messageList = entries.map((message) => { | ||
| const handleLikeClick = | ||
| typeof onHandleLike === 'function' | ||
| ? () => onHandleLike(message.id) | ||
| : undefined; | ||
|
Comment on lines
+8
to
+11
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. It is generally considered against best practices to create a function we will be passing to children components inside of the mapping function itself. Thinking about the single responsibility principle: we are doing two things at once by both defining functionality for a button press and mapping over Looking at the principles of encapsulation and packaging related code in the same file: the functionality for handleLikeClick seems tied to the ChatEntry component. ChatLog defines this function but never uses it itself, it only references the function to pass it to ChatEntry. How can we refactor this code so that we are not defining functionality inside of a mapping function and so that the code lives with the component that will use it? |
||
|
|
||
| return ( | ||
| <li 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. Great use of the unique message ids for the key value! |
||
| <ChatEntry | ||
| sender={message.sender} | ||
| body={message.body} | ||
| timeStamp={message.timeStamp} | ||
| liked={message.liked} | ||
| onHandleLike={handleLikeClick} | ||
| /> | ||
| </li> | ||
| ); | ||
| }); | ||
|
|
||
| return ( | ||
| <ul className="chat-log"> | ||
| {messageList} | ||
| </ul> | ||
| ); | ||
| }; | ||
|
|
||
|
|
||
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf(PropTypes.shape({ | ||
|
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. Great use of |
||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool.isRequired, | ||
| })).isRequired, | ||
| onHandleLike: PropTypes.func, | ||
| }; | ||
|
|
||
| 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.
I like this pattern of sending just the
idtohandleLikesince it keeps all the state management and message object creation confined toApp.