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
16 changes: 10 additions & 6 deletions src/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,30 @@
display: inline-block
}

#App .widget button {
border-radius: 80%;
}

.red {
color: #b22222
color: #b22222;
}

.orange {
color: #e6ac00
color: #e6ac00;
}

.yellow {
color: #e6e600
color: #e6e600;
}

.green {
color: green
color: green;
}

.blue {
color: blue
color: blue;
}

.purple {
color: purple
color: purple;
}
79 changes: 75 additions & 4 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,87 @@
import React from 'react';
import React, { useState } from 'react';
import './App.css';
import ChatLog from './components/ChatLog';
import chatMessages from './data/messages.json';
import ChatMessage from './models/ChatMessage';
import ColorChoice from './components/ColorChoice';

const messages = [];
let isLocal = true;
let initialLikes = 0;
for (const msg of chatMessages) {
messages.push(
new ChatMessage(
msg.id,
msg.sender,
msg.body,
msg.timeStamp,
msg.liked,
isLocal

Choose a reason for hiding this comment

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

Neat way to add the local/remote data to the messages. In this case where we only have 2 senders, I might suggest using the value of sender to determine remote vs local over holding a new piece of data, but this approach will scale better.

)
);
if (msg.liked) {
++initialLikes;
}
isLocal = !isLocal;
}

const App = () => {
const [chatEntries, setChatEntries] = useState(messages);
const [localColor, setLocalColor] = useState('');
const [remoteColor, setRemoteColor] = useState('');
Comment on lines +30 to +31

Choose a reason for hiding this comment

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

If both local and remote start out with black as the default color, that could be the initial value for these state variables.

const [totalLikes, setTotalLikes] = useState(initialLikes);

Choose a reason for hiding this comment

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

Since the liked status of a message lives in the chatEntries data we should avoid holding an extra piece of state that we need to manually keep in sync. We can use a higher order function like array.reduce to take our list of messages and reduce it down to a single value.

// This could be returned from a helper function
// totalLikes is a variable that accumulates a value as we loop over each entry in chatEntries
const likesCount = chatEntries.reduce((totalLikes, currentMessage) => {
    // If currentMessage.liked is true add 1 to totalLikes, else add 0
    return (totalLikes += currentMessage.liked ? 1 : 0);
}, 0); // The 0 here sets the initial value of totalLikes to 0


const updateChatEntry = (updatedEntry) => {
const updatedMsg = chatEntries.map((entry) => {
if (entry.id === updatedEntry.id) {
if (updatedEntry.liked) {
setTotalLikes(totalLikes + 1);
} else {
setTotalLikes(totalLikes - 1);
}
return updatedEntry;
} else return entry;
});
setChatEntries(updatedMsg);
};

const getColor = (local) => {
if (local) {
return localColor;
}
return remoteColor;
};

return (
<div id="App">
<header>
<h1>Application title</h1>
<h1>
Chat Between{' '}
<span className={localColor}>{chatEntries[0].sender}</span> and{' '}

Choose a reason for hiding this comment

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

Nice use of new lines to keep the contents of the h1 clear.

<span className={remoteColor}>{chatEntries[1].sender}</span>
</h1>
<section>
<ColorChoice
updateColor={setLocalColor}
senderName={chatEntries[0].sender}
newColor={localColor}
></ColorChoice>
<h2 className="widget" id="heartWidget">
{totalLikes} ❤️s
</h2>
<ColorChoice
updateColor={setRemoteColor}
senderName={chatEntries[1].sender}
newColor={remoteColor}
></ColorChoice>
</section>
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog
entries={chatEntries}
onUpdateChatEntry={updateChatEntry}
getColor={getColor}
></ChatLog>
</main>
</div>
);
Expand Down
40 changes: 32 additions & 8 deletions src/components/ChatEntry.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,46 @@
import React from 'react';
import './ChatEntry.css';
import PropTypes from 'prop-types';
import TimeStamp from './TimeStamp';
import ChatMessage from '../models/ChatMessage';

const ChatEntry = ({ message, onUpdateEntry, getColor }) => {
const typeHeart = message.liked ? '❤️' : '🤍';
const isLocalclass = message.isLocal ? 'local' : 'remote';
const colorClass = getColor(message.isLocal);

const clickLike = (e) => {
onUpdateEntry(
new ChatMessage(
message.id,
message.sender,
message.body,
message.timeStamp,
!message.liked,
message.isLocal
)
);
};
Comment on lines +11 to +22

Choose a reason for hiding this comment

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

I would consider passing the id of the message clicked to onUpdateEntry and having the App code handle the new message creation. When ChatEntry creates the new object for the App state, it takes some responsibility for managing those contents. If we want the responsibility of managing the state to live solely with App, we would want it to handle defining the new message object.

This made me think of a related concept in secure design for APIs. Imagine we had an API for creating and updating messages, and it has an endpoint /<msg_id>/like meant to update a true/false liked value. We could have that endpoint accept a body in the request and let the user send an object with data for the message's record (similar to passing a message object from ChatEntry to App), but the user could choose to send any data for those values. If the endpoint only takes in an id and handles updating the liked status for the message itself, there is less opportunity for user error or malicious action.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I understand!


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

ChatEntry.propTypes = {
//Fill with correct proptypes
message: PropTypes.instanceOf(ChatMessage).isRequired,
onUpdateEntry: PropTypes.func.isRequired,
getColor: PropTypes.func,
};

export default ChatEntry;
29 changes: 18 additions & 11 deletions src/components/ChatEntry.test.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
import React from "react";
import "@testing-library/jest-dom/extend-expect";
import ChatEntry from "./ChatEntry";
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
import React from 'react';
import '@testing-library/jest-dom/extend-expect';
import ChatEntry from './ChatEntry';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import ChatMessage from '../models/ChatMessage';

describe("Wave 01: ChatEntry", () => {
describe('Wave 01: ChatEntry', () => {
beforeEach(() => {
render(
<ChatEntry
sender="Joe Biden"
body="Get out by 8am. I'll count the silverware"
timeStamp="2018-05-18T22:12:03Z"
message={
new ChatMessage(
1,
'Joe Biden',
"Get out by 8am. I'll count the silverware",
'2018-05-18T22:12:03Z',
false
)
}
/>
);
});

test("renders without crashing and shows the sender", () => {
test('renders without crashing and shows the sender', () => {
expect(screen.getByText(/Joe Biden/)).toBeInTheDocument();
});

test("that it will display the body", () => {
test('that it will display the body', () => {
expect(screen.getByText(/Get out by 8am/)).toBeInTheDocument();
});

test("that it will display the time", () => {
test('that it will display the time', () => {
expect(screen.getByText(/\d+ years ago/)).toBeInTheDocument();
});
});
4 changes: 4 additions & 0 deletions src/components/ChatLog.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
margin: auto;
max-width: 50rem;
}

.chat-entries-list {
list-style: none;
}
33 changes: 33 additions & 0 deletions src/components/ChatLog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';
import ChatEntry from './ChatEntry';
import './ChatLog.css';
import PropTypes from 'prop-types';
import ChatMessage from '../models/ChatMessage';

const ChatLog = (props) => {
const chatEntries = props.entries.map((entry) => {
return (
<li key={entry.id}>
<ChatEntry
message={entry}
onUpdateEntry={props.onUpdateChatEntry}
getColor={props.getColor}
></ChatEntry>
</li>
);
});

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

ChatLog.propTypes = {
entries: PropTypes.arrayOf(PropTypes.instanceOf(ChatMessage)).isRequired,
onUpdateChatEntry: PropTypes.func.isRequired,
//getColor: PropTypes.func,
};

export default ChatLog;
48 changes: 24 additions & 24 deletions src/components/ChatLog.test.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,49 @@
import React from "react";
import "@testing-library/jest-dom/extend-expect";
import ChatLog from "./ChatLog";
import { render, screen } from "@testing-library/react";
import React from 'react';
import '@testing-library/jest-dom/extend-expect';
import ChatLog from './ChatLog';
import { render, screen } from '@testing-library/react';

const LOG = [
{
sender: "Vladimir",
body: "why are you arguing with me",
timeStamp: "2018-05-29T22:49:06+00:00",
sender: 'Vladimir',
body: 'why are you arguing with me',
timeStamp: '2018-05-29T22:49:06+00:00',
},
{
sender: "Estragon",
body: "Because you are wrong.",
timeStamp: "2018-05-29T22:49:33+00:00",
sender: 'Estragon',
body: 'Because you are wrong.',
timeStamp: '2018-05-29T22:49:33+00:00',
},
{
sender: "Vladimir",
body: "because I am what",
timeStamp: "2018-05-29T22:50:22+00:00",
sender: 'Vladimir',
body: 'because I am what',
timeStamp: '2018-05-29T22:50:22+00:00',
},
{
sender: "Estragon",
body: "A robot.",
timeStamp: "2018-05-29T22:52:21+00:00",
sender: 'Estragon',
body: 'A robot.',
timeStamp: '2018-05-29T22:52:21+00:00',
},
{
sender: "Vladimir",
body: "Notabot",
timeStamp: "2019-07-23T22:52:21+00:00",
sender: 'Vladimir',
body: 'Notabot',
timeStamp: '2019-07-23T22:52:21+00:00',
},
];

describe("Wave 02: ChatLog", () => {
describe('Wave 02: ChatLog', () => {
beforeEach(() => {
render(<ChatLog entries={LOG} />);
});

test("renders without crashing and shows all the names", () => {
test('renders without crashing and shows all the names', () => {
[
{
name: "Vladimir",
name: 'Vladimir',
numChats: 3,
},
{
name: "Estragon",
name: 'Estragon',
numChats: 2,
},
].forEach((person) => {
Expand All @@ -56,7 +56,7 @@ describe("Wave 02: ChatLog", () => {
});
});

test("renders an empty list without crashing", () => {
test('renders an empty list without crashing', () => {
const element = render(<ChatLog entries={[]} />);
expect(element).not.toBeNull();
});
Expand Down
23 changes: 23 additions & 0 deletions src/components/ColorChoice.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
button.red {
background-color: #b22222;
}

button.orange {
background-color: #e6ac00;
}

button.yellow {
background-color: #e6e600;
}

button.green {
background-color: green;
}

button.blue {
background-color: blue;
}

button.purple {
background-color: purple;
}
Loading