Skip to content

Sphinx - Leidy Martinez C22 #38

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#App header section {
background-color: #e0ffff;
color: black;
}

#App .widget {
Expand Down
53 changes: 49 additions & 4 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,60 @@
import { useState } from 'react';
import './App.css';
import ChatLog from './components/ChatLog';
import messages from './data/messages.json';

const App = () => {

const calculateTotalLikeCount = (chat) => {
return chat.reduce((total, chat) => {
return total + (chat.liked ? 1 : 0);
}, 0);
};
Comment on lines +7 to +11

Choose a reason for hiding this comment

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

Excellent execution!


const localSender = messages[0].sender;
let remoteSender = 'Unknown';

for (const message of messages) {
if (message.sender !== localSender) {
remoteSender = message.sender;
break; // Stop once other sender is found
}
}
Comment on lines +13 to +21

Choose a reason for hiding this comment

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

Nice this handles cases of double texting!



const App =() => {
const [chat, setChat] = useState(messages);

Choose a reason for hiding this comment

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

Maybe we could change this from chat to chats since it is multiple chat messages instead of one?


const handleLikeButtom = (id) => {
setChat(chat => {
return chat.map(chat => {
if (chat.id === id) {
return { ...chat, liked: !chat.liked };
} else {
return chat;
}
});
});
};
Comment on lines +27 to +37

Choose a reason for hiding this comment

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

No notes on this! Shows me that you have a great understanding of lifting up state!

const totalLikes = calculateTotalLikeCount(chat);
return (
<div id="App">
<header>
<h1>Application title</h1>
<h1>Chat Between {localSender} and {remoteSender}</h1>
<section>
<h1>{`${totalLikes} ❤️s`}</h1>
</section>
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<div>{
<ChatLog
entries={chat}
onLiked={handleLikeButtom}
localSender={localSender}
likes={totalLikes} />}
Comment on lines +49 to +53

Choose a reason for hiding this comment

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

Does totalLikes actually need to be passed? We are only using it in line 44.


</div>
</main>

</div>
);
};
Expand Down
3 changes: 2 additions & 1 deletion src/components/ChatEntry.css
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,5 @@ button {

.chat-entry.remote .entry-bubble:hover::before {
background-color: #a9f6f6;
}
}

32 changes: 24 additions & 8 deletions src/components/ChatEntry.jsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,36 @@
import './ChatEntry.css';
import PropTypes from 'prop-types';
import TimeStamp from './TimeStamp';


const ChatEntry = (props) => {
const likeButtonCliked = () => {
props.onLiked(props.id);
};
Comment on lines +7 to +9

Choose a reason for hiding this comment

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

The button function could also be implemented like so:

<button className='like' onClick={() => likeButtonClicked(id)}>{liked ? '❤️': '🤍'}</button>


const heartColor = props.liked ? '❤️' : '🤍';
const messageClass = props.sender === props.localSender ? 'local' : 'remote';
Comment on lines +11 to +12

Choose a reason for hiding this comment

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

Ternaries, love to see them!


const ChatEntry = () => {
return (
<div className="chat-entry local">
<h2 className="entry-name">Replace with name of sender</h2>
<div className={`chat-entry ${messageClass}`}>
<h2 className="entry-name">{props.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>{props.body}</p>
<p className="entry-time"><TimeStamp time={props.timeStamp}></TimeStamp></p>
<button className='like' onClick={likeButtonCliked}>{heartColor}</button>
</section>
</div>
);
};

ChatEntry.propTypes = {
// Fill with correct proptypes
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
onLiked: PropTypes.func.isRequired,
localSender: PropTypes.string.isRequired
};

export default ChatEntry;
export default ChatEntry;
46 changes: 46 additions & 0 deletions src/components/ChatLog.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import ChatEntry from './ChatEntry';
import PropTypes from 'prop-types';


const ChatLog = (props) => {
const chatentrycomponents = props.entries.map((entries, index) => {

Choose a reason for hiding this comment

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

We could maybe name this entry instead of entries since it only a single entry we will be looking at each iteration.

return (
<ul key={index}>

Choose a reason for hiding this comment

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

Since we have the ids of entries we could actually use that. We want to avoid using the indexes.

<ChatEntry
id={entries.id}
sender={entries.sender}
body={entries.body}
timeStamp={entries.timeStamp}
liked={entries.liked}
onLiked={props.onLiked}
localSender={props.localSender}
></ChatEntry>
Comment on lines +9 to +17

Choose a reason for hiding this comment

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

Since the names of the keys on entry are the same as the names your are setting on ChatEntry attributes/you are using all of the values in entry you could do something something like this to save you a few keystrokes:

  const chatComponents = entries.map((entry) => {
    return(
      <ChatEntry
        {...entry}
        onLikeToggle={onLikeBtnToggle}
        key={entry.id}
      />
    );
  });

Just recognize that with this method it is not as explicit as what you have. Something to consider!

</ul>
);
});

return (
<section>
<ul className="chat-log">
{chatentrycomponents}
</ul>
</section>
);
};

ChatLog.propTypes = {
entries: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
})
).isRequired,

onLiked: PropTypes.func.isRequired,
localSender: PropTypes.string.isRequired,
Comment on lines +32 to +43

Choose a reason for hiding this comment

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

⭐️

Choose a reason for hiding this comment

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

I see that the tests are saying that some of the props are undefined but that isn't the case. I'm not entirely sure why.

};

export default ChatLog;