-
Notifications
You must be signed in to change notification settings - Fork 40
Dragonfly - Lina Martinez #30
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
13e4537
08b7a57
fbb4f12
b5626bd
d6db016
aa13a28
ed72cde
a195f16
253e6fb
7a08768
0dac9b0
36cd833
781f61e
39cc8a6
8e0b9a1
3fded1c
d0c6ae2
35952ed
44d8477
e733a5c
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,65 @@ | ||||||
| import './App.css'; | ||||||
| import ChatLog from './components/ChatLog'; | ||||||
| import ColorChoice from './components/ColorChoice'; | ||||||
| import DATA from './data/messages.json'; | ||||||
| import { useState } from 'react'; | ||||||
|
|
||||||
| const App = () => { | ||||||
| const [entryData, setEntryData] = useState(DATA); | ||||||
| const [localColor, setLocalColor] = useState('green'); | ||||||
| const [remoteColor, setRemoteColor] = useState('blue'); | ||||||
|
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. Instead of creating 2 separate pieces of state to track colors for the local user and the remote user, you could create just one piece of state that can keep track of both pieces of these info. const [usersColors, setUsersColors] = useState({
local: 'green',
remote: 'blue'
}); |
||||||
|
|
||||||
| const LOCAL_SENDER = entryData[0].sender; | ||||||
| const REMOTE_SENDER = entryData[1].sender; | ||||||
|
Comment on lines
+12
to
+13
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 works for our application right now! You'd need to come up with a different solution for how identifiying different senders if react-chatlog allowed for messages between more than 2 senders (like a big group chat between many friends). |
||||||
|
|
||||||
| const handleColorChange = (sender, color) => { | ||||||
| if (sender === LOCAL_SENDER){ | ||||||
| setLocalColor(color); | ||||||
| } else if (sender === REMOTE_SENDER) { | ||||||
| setRemoteColor(color); | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| const updateEntryLikedState = (entryId) => { | ||||||
| setEntryData(entries => { | ||||||
|
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 callback setter style. In this application, it doesn't really matter whether we use the callback style or the value style, but it's good practice to get in the habit of using the callback style. |
||||||
| return 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 === entryId) { | ||||||
| return { ...entry, liked: !entry.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 entry; | ||||||
| } | ||||||
| }); | ||||||
| }); | ||||||
| }; | ||||||
|
|
||||||
| const totalLikes = entryData.reduce((sum, entry) => { | ||||||
| return entry.liked ? sum + 1 : sum; | ||||||
|
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 Instead of using a ternary operator here, we could write something like:
Suggested change
Adding a boolean to a number treats true as 1 and 0 as false |
||||||
| }, 0); | ||||||
|
|
||||||
| return ( | ||||||
| <div id="App"> | ||||||
| <header> | ||||||
| <h1>Application title</h1> | ||||||
| <h1>Chat Between{' '} | ||||||
| <span className={localColor}>{LOCAL_SENDER}</span>{' '} | ||||||
| and{' '} | ||||||
| <span className={remoteColor}>{REMOTE_SENDER}</span> | ||||||
|
Comment on lines
+43
to
+45
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. 👍 |
||||||
| </h1> | ||||||
|
|
||||||
| <section> | ||||||
| <ColorChoice sender={LOCAL_SENDER} chatColor={localColor} setColorCallback={handleColorChange} /> | ||||||
| <span id='heartWidget' className='widget'>{totalLikes} ❤️s</span> | ||||||
| <ColorChoice sender={REMOTE_SENDER} chatColor={remoteColor} setColorCallback={handleColorChange} /> | ||||||
| </section> | ||||||
| </header> | ||||||
|
|
||||||
| <main> | ||||||
| {/* Wave 01: Render one ChatEntry component | ||||||
| Wave 02: Render ChatLog component */} | ||||||
| <ChatLog | ||||||
| entries={entryData} | ||||||
| onToggleHeart={updateEntryLikedState} | ||||||
| chatLocalColor={localColor} | ||||||
| chatRemoteColor={remoteColor} | ||||||
| localSender={LOCAL_SENDER} | ||||||
| /> | ||||||
| </main> | ||||||
| </div> | ||||||
| ); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,36 @@ | ||
| import './ChatEntry.css'; | ||
| import TimeStamp from './TimeStamp'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatEntry = ({ id, sender, body, timeStamp, liked, onToggleLike, isLocal, chatColor }) => { | ||
| const entryClass = `chat-entry ${isLocal ? 'local' : 'remote'}`; | ||
|
|
||
| const ChatEntry = () => { | ||
| return ( | ||
| <div className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <div className={entryClass}> | ||
| <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 className={chatColor}>{body}</p> | ||
| <p className="entry-time"><TimeStamp time={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={() => { onToggleLike(id); }} | ||
| > | ||
| {liked ? '❤️' : '🤍'} | ||
| </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, | ||
| onToggleLike: PropTypes.func.isRequired, | ||
| isLocal: PropTypes.bool.isRequired, | ||
| chatColor: PropTypes.string.isRequired | ||
| }; | ||
|
|
||
| export default ChatEntry; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import ChatEntry from './ChatEntry'; | ||
| import './ChatLog.css'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatLog = ({ entries, onToggleHeart, chatLocalColor, chatRemoteColor, localSender}) => { | ||
| const chatEntryComponents = entries.map((entry) => { | ||
| const isLocal = entry.sender === localSender; | ||
| const chatColor = isLocal ? chatLocalColor : chatRemoteColor; | ||
| return ( | ||
| <ChatEntry | ||
| key={entry.id} | ||
| id={entry.id} | ||
| sender={entry.sender} | ||
| body={entry.body} | ||
| timeStamp={entry.timeStamp} | ||
| liked={entry.liked} | ||
| onToggleLike={onToggleHeart} | ||
| isLocal={isLocal} | ||
| chatColor={chatColor} | ||
| ></ChatEntry> | ||
| ); | ||
| }); | ||
|
|
||
| return ( | ||
| <div className='chat-log'> | ||
| {chatEntryComponents} | ||
| </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.isRequired, | ||
| }) | ||
| ), | ||
| onToggleHeart: PropTypes.func.isRequired, | ||
| chatLocalColor: PropTypes.string.isRequired, | ||
| chatRemoteColor: PropTypes.string.isRequired, | ||
| localSender: PropTypes.string.isRequired | ||
| }; | ||
|
|
||
| export default ChatLog; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| label { | ||
| font-size: 1.1rem; | ||
| font-weight: 600; | ||
| } | ||
| select { | ||
| margin-left: 1rem; | ||
| cursor: pointer; | ||
| border-radius: 10px; | ||
| font-size: 1rem; | ||
| padding: 0.5rem; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import PropTypes from 'prop-types'; | ||
| import './ColorChoice.css'; | ||
|
|
||
| const ColorChoice = ({sender, chatColor, setColorCallback}) => { | ||
| const handleColorChange = (event) => { | ||
| setColorCallback(sender, event.target.value); | ||
| }; | ||
|
|
||
| return ( | ||
| <label className={chatColor}> | ||
| {sender}'s color: | ||
| <select name="colorSelect" defaultValue="chooseColor" onChange={handleColorChange}> | ||
| <option value="">Choose a color</option> | ||
| <option value="red" >🔴 Red</option> | ||
| <option value="orange">🟠 Orange</option> | ||
| <option value="brown">🟤 Brown</option> | ||
| <option value="green">🟢 Green</option> | ||
| <option value="blue">🔵 Blue</option> | ||
| <option value="purple">🟣 Purple</option> | ||
| </select> | ||
| </label> | ||
| ); | ||
| }; | ||
|
|
||
| ColorChoice.propTypes = { | ||
| sender: PropTypes.string.isRequired, | ||
| chatColor: PropTypes.string.isRequired, | ||
| setColorCallback: PropTypes.func.isRequired | ||
| }; | ||
| export default ColorChoice; |
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.
🤩👍