Skip to content

Conversation

@J-J-D
Copy link

@J-J-D J-J-D commented Jun 22, 2022

Otters - Jodi Denney

Copy link

@goeunpark goeunpark left a comment

Choose a reason for hiding this comment

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

Wonderful work, Jodi! 🎉

Your tests pass, your props cascade down beautifully, and your App-level state is managed delightfully! This code is so clean and well-thought out; it is a well-deserved Green. 🟢

Keep it up! 🚀

import './App.css';
import chatMessages from './data/messages.json';
import ChatLog from './components/ChatLog';
import { useState } from 'react';

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

Comment on lines +7 to +8
const [entries, setEntries] = useState(chatMessages);
const [likes, setLikes] = useState(0);

Choose a reason for hiding this comment

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

So organized, setting all the state variables at the top of the component! Love to see it 💖

Comment on lines +22 to +23
setEntries(newEntries);
setLikes(currentLikes);

Choose a reason for hiding this comment

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

Oooo, so efficient. ✨

};

ChatLog.propTypes = {
entries: PropTypes.array.isRequired,

Choose a reason for hiding this comment

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

We should also add toggleHeartCallback as a required prop type.

);
});

return <ul>{chatComponents}</ul>;

Choose a reason for hiding this comment

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

Love that you used semantic HTML with <ul> and <li> here!

const ChatEntry = (props) => {
const toggleHeart = () => {
props.toggleHeartCallback(props.id);
};

Choose a reason for hiding this comment

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

Small nit, a space after this life would help with readability!

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