Skip to content
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
4 changes: 1 addition & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"dependencies": {
"luxon": "^2.5.2",
"prop-types": "^15.8.1",
"react": "^18.3.1",
"react-dom": "^18.3.1"
},
Expand Down
14 changes: 14 additions & 0 deletions src/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,18 @@

.purple {
color: purple
}

.color-controls {
background: rgb(173, 238, 238);
display: flex;
justify-content: space-around;
align-items: center;
padding: 1rem;
}

.color-picker {
display: flex;
align-items: center;
gap: 0.5rem;
}
58 changes: 55 additions & 3 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,66 @@
import './App.css';
import DATA from './data/messages.json';
import ChatLog from './components/ChatLog';
import ColorChoice from './components/ColorChoice';
import { useState } from 'react';


const handleLikedChat = chat => {
return { ...chat, liked: !chat.liked};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return { ...chat, liked: !chat.liked};
return { ...chat, liked: !chat.liked };

};

const countLikes = chatData => {
return chatData.reduce((sum, chat) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Though you will find a number of implementations for this, the .reduce method is the one I would prefer for the given case.

return sum + (chat.liked ? 1: 0);
}, 0);
Comment on lines +8 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on these functions, especially with having them defined outside of the component since they aren't dependent on the locally scoped data like state.

};

const App = () => {
const [chatData, setChatData] = useState(DATA);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

const [localColor, setLocalColor] = useState('');
const [remoteColor, setRemoteColor] = useState('');
Comment on lines +20 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perhaps there is a way to combine these two pieces of state into a single piece so we don't have manage 3 separate states. 👀

@tsztin0217


const handleLikeClicked = id => {
setChatData(chatData => {
return chatData.map(chat => {
if (chat.id === id) {
return handleLikedChat(chat);
} else {
return chat;
Comment on lines +26 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Given the relative simplicity here, you could implement a ternary—foregoing the conditional branching which would result in more succinct code yet maintaining the same level of readability.

}
});
});
};

const totalLikes = countLikes(chatData);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍🏿


return (
<div id="App">
<header>
<h1>Application title</h1>
<h1>
Chat between <span className={localColor}>Vladimir</span> and <span className={remoteColor}>Estragon</span>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Currently, you have the names of the chat participants hardcoded, how would we refactor this to make it more dynamic and interchangeable?

</h1>
<div className="color-controls">
<div className="color-picker">
<span className={localColor}><b>Vladimir's color:</b> </span>
<ColorChoice onClickColor={setLocalColor} />
</div>
<div>
<h2>{totalLikes} ❤️s</h2>
</div>
<div className="color-picker">
<span className={remoteColor}><b>Estragon's color:</b> </span>
<ColorChoice onClickColor={setRemoteColor} />
Comment on lines +46 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

</div>
</div>
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog
entries={chatData}
onLikeToggle={handleLikeClicked}
Comment on lines +59 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

localColor={localColor}
remoteColor={remoteColor}
Comment on lines +61 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/>
</main>
</div>
);
Expand Down
37 changes: 27 additions & 10 deletions src/components/ChatEntry.jsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,38 @@
import './ChatEntry.css';
import TimeStamp from './TimeStamp';
import PropTypes from 'prop-types';

const ChatEntry = (props) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't forget that you can de-structure your props here! Though this does readily display that you are working with props and not local variables declared in the components.


const likeButtonClicked = () => {
props.onLikeToggle(props.id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍🏿

};

const likeButtonColor = props.liked ? '❤️' : '🤍';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of a ternary!

const isLocal = props.sender === 'Vladimir';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For the same reasons mentioned in App.jsx, you wouldn't want to hardcode this data. How could it be made more dynamic? What if the senders are no longer Vladimir and Estragon?

const colorClass = isLocal ? props.localColor : props.remoteColor;

const ChatEntry = () => {
return (
// Replace the outer tag name with a semantic element that fits our use case
<replace-with-relevant-semantic-element className="chat-entry local">
<h2 className="entry-name">Replace with name of 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>
<article className={`chat-entry ${isLocal ? 'local' : 'remote'}`}>
<h2 className="entry-name">{props.sender}</h2>
<section className={`entry-bubble ${colorClass}`}>
<p>{props.body}</p>
<p className="entry-time"><TimeStamp time={props.timeStamp}></TimeStamp></p>
<button className="like" onClick={likeButtonClicked}>{likeButtonColor}</button>
Comment on lines +16 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

</section>
</replace-with-relevant-semantic-element>
</article>
);
};

ChatEntry.propTypes = {
// Fill with correct proptypes
sender: PropTypes.string,
body: PropTypes.string,
timeStamp: PropTypes.string,
liked: PropTypes.bool,
id: PropTypes.number,
onLikeToggle: PropTypes.func,
localColor: PropTypes.string,
remoteColor: PropTypes.string,
Comment on lines +28 to +35
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work on these proptypes, remember that they have been sunsetted in React and that will be using other type checking frameworks like Typescript.

};

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

const ChatLog = (props) => {
const chatComponents = props.entries.map((entry) => {
return (<ChatEntry
key={entry.id}
id={entry.id}
sender={entry.sender}
body={entry.body}
timeStamp={entry.timeStamp}
liked={entry.liked}
onLikeToggle={props.onLikeToggle}
localColor={props.localColor}
remoteColor={props.remoteColor}
Comment on lines +7 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work, especially on the formatting. To simplify your logic, you could pass in the entire message object yourself. However, you would lose out on the explicitness you have here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As for localColor and remoteColor, you are currently passing in a piece of data to every entry that won't be used. If it is a local message, it won't use the remote color and vice versa. What you could do instead is have a generalized color prop and then use a ternary do decide which of the colors, remote or local, should be passed based on the sender of the message. @tsztin0217

/>
);
});

return <ul className="chat-log">{chatComponents}</ul>;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

};

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,
})).isRequired,
onLikeToggle: PropTypes.func.isRequired,
localColor: PropTypes.string.isRequired,
remoteColor: PropTypes.string.isRequired,
};
Comment on lines +23 to +34
Copy link
Copy Markdown

Choose a reason for hiding this comment

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


export default ChatLog;
14 changes: 14 additions & 0 deletions src/components/ColorChoice.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const ColorChoice = (props) => {
return (
<>
<button className='red' onClick={() => props.onClickColor('red')}>🔴</button>
<button className='orange' onClick={() => props.onClickColor('orange')}>🟠</button>
<button className='yellow' onClick={() => props.onClickColor('yellow')}>🟡</button>
<button className='green' onClick={() => props.onClickColor('green')}>🟢</button>
<button className='blue' onClick={() => props.onClickColor('blue')}>🔵</button>
<button className='purple' onClick={() => props.onClickColor('purple')}>🟣</button>
</>
Comment on lines +3 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmmm, looks like there is a lot of repeating oneself here... how could this be DRY'd up? 🤔

);
};

export default ColorChoice;