Sphinx - Vlada Rapaport

[vladarap88]

No description provided.

[yangashley]

Prefer not to have comments in code unless absolutely necessary.

Suggested change

	const [counter, setCounter] = useState(0); // Initial counter set to 0
	const [counter, setCounter] = useState(0);

[yangashley]

Per the pattern we did during the flasky-front-end livecode and what we hope to see in the task-list-front-end activity, we want state to keep track of ChatEntrys here. Or in flasky-front-end, the state in App.jsx kept track of individual Cats and in task-list-front-end the state in App.jsx should keep track of Tasks. We want our whole app to know that a single component has been updated so we had to lift that state up to App.jsx.

You haven't lifted state up from the individual ChatEntry up into App in this project. You do use state, but we don't need state to keep track of a count of the hearts. We can calculate this value from each ChatEntry. If a value can be calculated, then we should prefer not to use state to handle this logic.

We want App.jsx to know a change has happened to ChatEntry so that React knows to re-render the each ChatEntry.

This is the state we'd like to see implemented.

  const [chatMessages, setChatMessages] = useState(messages); 

[yangashley]

It's more typical to see the result of using map referenced by a variable and using that variable in the JSX, like:

    const chatEntries = entries.map((entry) => {
	      return (
	          <ChatEntry                                 
	            key={entry.id}
	            id={entry.id}
	            sender={entry.sender}
	            body={entry.body}
	            timeStamp={entry.timeStamp}
	            liked={entry.liked}
	            handleLike={handleLike}
	            currentUser={currentUser}
	          />
	        );
	     });
    };

  return (
        <div>
           {chatEntries}
        </div>
  );

[yangashley]

Suggested change

	const ChatLog = ({entries, onLike, onUnlike}) => {
	const ChatLog = ({ entries, onLike, onUnlike }) => {

[yangashley]

Suggested change

	const ChatEntry = ({sender, body, timeStamp, onLike, onUnlike}) => {
	const ChatEntry = ({ sender, body, timeStamp, onLike, onUnlike }) => {

Should have white spaces around curly braces when using destructuring syntax.

I'd also expect ChatLog to pass the value of liked to each ChatEntry so that this component knows if liked is true or false to set a heart color.

[yangashley]

Suggested change

	<p className="entry-time"><TimeStamp time ={timeStamp} /></p>
	<p className="entry-time"><TimeStamp time={timeStamp} /></p>

Nice job using the provided Timestamp component here.

Convention is to not have spaces around = sign when passing props

[yangashley]

We shouldn't need to use state to keep track of the heart because we can get liked from each message. We should compute whether the heart is red or white by looking at messages liked value. We should prefer to not introduce state, which introduces additional complexity into a project, if we have some other way to accomplish the feature.

[yangashley]

What I'd like to see I'd like to see is an event handler written in app and this event handler would be passed down through props from App to ChatLog and finally down to ChatEntry.

ChatEntry could use this event handler passed down as a prop by registering it to the <button> element.

This is similar to how we implemented the pet-a-cat button in the Cat component in flasky-front-end. Have a look at App.jsx here from lines 44-55 to see how we write the event handler. Also have a look at Cat.jsx here from lines 17-21 to see how the passed down event handler gets used.

[yangashley]

Instead of two methods for handling liking and unliking a ChatEntry, you can write an event handler that handles both scenarios.

That would look something like having an event handler that changes the state of the ChatEntrys from App and then using another helper function that looks at the state of all the ChatEntries to calculate the heart counts:

  const handleLikeData = (id) => {
    const updatedEntries = chatMessages.map(chatMessage => {
      if (chatMessage.id === id) {
        return { ...chatMessage, liked: !chatMessage.liked };
      }
      else {
        return chatMessage;
      }
    })
    setChatMessages(updatedEntries);
  };

  const calculateTotalLikes = () => 
    return chatMessages.reduce((totalLikes, chatMessage) => (totalLikes + chatMessage.liked), 0);
  };

You can see how we do this with pets on a Cat in flasky here.