Skip to content
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.

Sign up for GitHub

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

Ocelots - Eva Liu #139

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Ocelots - Eva Liu #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d31754
wave1 and wave2 showwing all datas and right time format.
wich229 Jan 28, 2023
b7cef87
heart button and add count section
wich229 Jan 29, 2023
49c9b1d
wave 3 and all test finished.
wich229 Feb 1, 2023
31012ce
Merge pull request #1 from wich229/state_lifting_test
wich229 Feb 1, 2023
f4ce65e
add local and remote.
wich229 Feb 2, 2023
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
32 changes: 22 additions & 10 deletions src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,47 +28,59 @@

#App header section {
background-color: #e0ffff;
color: black;
height: 4rem;
font-size: 1.25rem;
display: flex;
justify-content: center;
align-items: center;
}

#App header section span {
font-size: 1.25rem;
color: rgba(100%, 0%, 0%, 0);
text-shadow: 0 0 0 red;
}

#App .widget {
display: inline-block;
line-height: 0.5em;
border-radius: 10px;
color: black;
font-size:0.8em;
font-size: 0.8em;
padding-left: 1em;
padding-right: 1em;
}

#App #heartWidget {
font-size: 1.5em;
margin: 1em
margin: 1em;
}

#App span {
display: inline-block
display: inline-block;
}

.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;
}
24 changes: 21 additions & 3 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,34 @@
import React from 'react';
import './App.css';
import ChatLog from './components/ChatLog';
import chatMessages from './data/messages.json';
import React, { useState } from 'react';

const App = () => {
const [chatsData, setChatsData] = useState(chatMessages);

const updateChatsData = (updateChat) => {
const chats = chatsData.map((chat) => {
if (chat.id === updateChat.id) {
return updateChat;
} else {
return chat;
}
});
setChatsData(chats);
};

const heartCounts = chatsData.filter((chat) => chat.liked === true);
Copy link

Choose a reason for hiding this comment

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

Great work calculating the hearts count from the chatsData! Since we don't need the contents of the array we get from filter, another option is to use a higher order function like array.reduce to take our list of messages and reduce it down to a single value.

// totalLikes is a variable that accumulates a value as we loop over each entry in chatEntries
const heartCounts = chatsData.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


return (
<div id="App">
<header>
<h1>Application title</h1>
<section>
<p>{heartCounts.length} ❤️s</p>
</section>
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog entries={chatsData} updateChatsData={updateChatsData} />
</main>
</div>
);
Expand Down
23 changes: 16 additions & 7 deletions src/components/ChatEntry.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
button {
background: none;
color: inherit;
border: none;
padding: 10px;
font: inherit;
cursor: pointer;
outline: inherit;
color: inherit;
border: none;
padding: 10px;
font: inherit;
cursor: pointer;
outline: inherit;
}

.chat-entry {
Expand Down Expand Up @@ -97,4 +97,13 @@ button {

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

.like {
color: white;
}

.liked {
color: rgba(100%, 0%, 0%, 0);
text-shadow: 0 0 0 red;
}
38 changes: 32 additions & 6 deletions src/components/ChatEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,48 @@
//Presentational Component
import React from 'react';
import './ChatEntry.css';
import TimeStamp from './TimeStamp';
import PropTypes from 'prop-types';

const ChatEntry = (props) => {
const changeLikeData = () => {
const chatData = {
id: props.id,
sender: props.sender,
body: props.body,
timeStamp: props.timeStamp,
liked: !props.liked,
};
props.updateChatsData(chatData);
};
Comment on lines +8 to +17
Copy link

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 props.updateChatsData and having the App code handle the new object 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.

Hi Kesley,
regarding the secure design for APIs topic, will that also apply to the PUT and PATCH routes? I was trying on these two when I was building up the like counts for the inspiration board, and I realized that the PATCH from the front-end needed to add the request body, but PUT didn't. I tried and both worked as the expected result.

And I read another answer from the stack Overflow (not sure if it's correct), the PUT will update all data and the PATCH will only update the one that has been selected. I wasn't sure which way is better approach.

Thank you!

Copy link

Choose a reason for hiding this comment

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

This does also apply to PATCH where we're typically only updating one or some of the values. In that case we have to send some data for the values that are changing, but we should choose to only let folks send the relevant data, rather than data for the entire object. When it comes to PUT, we're generally replacing an entire resource so we need to send all the data for the object. In public applications we want to do a lot of data sanitization when taking in input from users to ensure that it is only the types that are allowed and that we escape special characters which can help prevent running malicious code sent in place of text.

//I was did the recolor heart emoji first.
// const heartColor = props.liked ? 'liked' : 'like';

const heartColor = props.liked ? '❤️' : '🤍';
const messageType = props.sender === 'Vladimir' ? 'local' : 'remote';

return (
<div className="chat-entry local">
<h2 className="entry-name">Replace with name of sender</h2>
<div className={`chat-entry ${messageType}`}>
<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} />
</p>
<button className="like" onClick={changeLikeData}>
{heartColor}
</button>
</section>
</div>
);
};

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

export default ChatEntry;
37 changes: 37 additions & 0 deletions src/components/ChatLog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//Container Component

import React from 'react';
import ChatEntry from './ChatEntry';
import PropTypes from 'prop-types';

const ChatLog = (props) => {
const getChatEntry = props.entries.map((entrie) => {
return (
<ChatEntry
key={entrie.id}
id={entrie.id}
sender={entrie.sender}
body={entrie.body}
timeStamp={entrie.timeStamp}
liked={entrie.liked}
updateChatsData={props.updateChatsData}
/>
);
});
return <section>{getChatEntry}</section>;
};

ChatLog.propTypes = {
entries: PropTypes.arrayOf(
Copy link

Choose a reason for hiding this comment

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

Really nice use of PropTypes.

PropTypes.shape({
id: PropTypes.number.isRequired,
sender: PropTypes.string.isRequired,
body: PropTypes.string.isRequired,
timeStamp: PropTypes.string.isRequired,
liked: PropTypes.bool.isRequired,
}).isRequired
),
updateChatsData: PropTypes.func.isRequired,
};

export default ChatLog;
54 changes: 30 additions & 24 deletions src/components/ChatLog.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,55 @@
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 = [
// need to add id from the message.json
{
sender: "Vladimir",
body: "why are you arguing with me",
timeStamp: "2018-05-29T22:49:06+00:00",
id: 1,
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",
id: 2,
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",
id: 3,
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",
id: 4,
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",
id: 5,
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 +62,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