Skip to content

Dragonfly - Lina Martinez#30

Open
linakl19 wants to merge 20 commits intoAda-C23:mainfrom
linakl19:main
Open

Dragonfly - Lina Martinez#30
linakl19 wants to merge 20 commits intoAda-C23:mainfrom
linakl19:main

Conversation

@linakl19
Copy link

No description provided.

linakl19 added 20 commits June 10, 2025 23:18
Copy link

@yangashley yangashley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job on react-chatlog!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩👍

Comment on lines +12 to +13
const LOCAL_SENDER = entryData[0].sender;
const REMOTE_SENDER = entryData[1].sender;

Choose a reason for hiding this comment

The 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).

Comment on lines +9 to +10
const [localColor, setLocalColor] = useState('green');
const [remoteColor, setRemoteColor] = useState('blue');

Choose a reason for hiding this comment

The 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 updateEntryLikedState = (entryId) => {
setEntryData(entries => {

Choose a reason for hiding this comment

The 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.


const updateEntryLikedState = (entryId) => {
setEntryData(entries => {
return entries.map(entry => {

Choose a reason for hiding this comment

The 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.

setEntryData(entries => {
return entries.map(entry => {
if (entry.id === entryId) {
return { ...entry, liked: !entry.liked };

Choose a reason for hiding this comment

The 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.

};

const totalLikes = entryData.reduce((sum, entry) => {
return entry.liked ? sum + 1 : sum;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job using reduce! This is how most JS devs would go about calculating a total amount too.

Instead of using a ternary operator here, we could write something like:

Suggested change
return entry.liked ? sum + 1 : sum;
(totalLikes + message.liked)

Adding a boolean to a number treats true as 1 and 0 as false

Comment on lines +43 to +45
<span className={localColor}>{LOCAL_SENDER}</span>{' '}
and{' '}
<span className={remoteColor}>{REMOTE_SENDER}</span>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<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>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job using the provided TimeStamp component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments