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

Sharks - Camilla #107

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

Sharks - Camilla #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d42ffd4
Modifies all files, implements components
camilla122333 Jun 27, 2022
98817e7
Adds ChatLog component
camilla122333 Jun 27, 2022
776afb5
Cleans up comments
camilla122333 Jun 27, 2022
a555afc
Changes emoji for testing
camilla122333 Jun 27, 2022
b7a8cec
Removes debugging comments, minor changes
camilla122333 Jun 27, 2022
f774c6d
Changes key to the unique value of timeStamp, to get rid of the test …
camilla122333 Jun 27, 2022
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
23 changes: 23 additions & 0 deletions src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
background-color: #87cefa;
}


#App header {
background-color: #222;
color: #fff;
Expand All @@ -13,6 +14,20 @@
align-items: center;
}

/* #App span {
background-color: violet;
color: #fff;
padding-bottom: 0.5rem;
position: fixed;
width: 100%;
height: 120px;
z-index: 100;
text-align: center;
align-items: center;
display: flex;
flex-direction: column;
} */

#App main {
padding-left: 2em;
padding-right: 2em;
Expand All @@ -30,6 +45,14 @@
background-color: #e0ffff;
}

#App .header {
font-size: 1.2em;
text-align: center;
font-weight: bold;
font-family: sans-serif;
height: 20px;
}

#App .widget {
display: inline-block;
line-height: 0.5em;
Expand Down
50 changes: 44 additions & 6 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,54 @@
import React from 'react';
import React, { useState } from 'react';
import './App.css';
import chatMessages from './data/messages.json';
import ChatLog from './components/ChatLog';

const App = () => {
const [chatMessagesData, setChatMessagesData] = useState(chatMessages);
const users = [];
let likesCount = 0;
let user1 = '';
let user2 = '';

const updateChatData = (updatedMessage) => {
const messages = chatMessagesData.map((message) => {
if (message.id === updatedMessage.id) {
return updatedMessage;
} else {
return message;
}
});

setChatMessagesData(messages);
};
Comment on lines +13 to +23
Copy link

Choose a reason for hiding this comment

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

We use this method to update how a chat entry renders after someone likes an entry. To constrain the scope of this method so that it is only responsible for re-rendering a chat entry after it's been liked, we might refactor this method so that we're not returning the entire message.

updateChatData could become something like:

const updateHeart = (id) => {
    const chatMessagesCopy = [...chatMessagesData];
    for (let entry of chatMessagesCopy) {
      if (entry.id === id) {
        entry.heart = !entry.heart; 
      }
    }
    setChatMessagesData(chatMessagesCopy);
  };

If you do this, then you can update ChatEntry.js too so you only need to pass the prop id instead of the whole message object


for (let i = 0; i < chatMessagesData.length; i++) {
let message = chatMessagesData[i];
if (users.indexOf(message['sender']) === -1) {
users.push(message['sender'])
}
if (message['liked']) {
likesCount++;
}
}
Comment on lines +25 to +33
Copy link

Choose a reason for hiding this comment

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

Putting this logic into a method like calculateHeartCount and then calling that function within the paragraph tag would help make this file a little more organized and readable.

<p className="header">{calculateHeartCount()} 💙s</p>

Having said that, your logic as it is right now would need to be reworked so that when the method is called it would return an integer


if (users.length === 2) {
user1 = users[0].toUpperCase();
user2 = users[1].toUpperCase();
}
Comment on lines +35 to +38
Copy link

Choose a reason for hiding this comment

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

You could also directly hard code these user names into your h1 element and remove this logic here so line 44 would look like:

<h1>CHAT BETWEEN {{chatMessages[0].sender}} AND {{chatMessages[1].sender}} </h1>


return (
<div id="App">
<header>
<h1>Application title</h1>
</header>

<header>
<h1>CHAT BETWEEN {user1} AND {user2} </h1>
<p className="header">{likesCount} 💙s</p>
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog
entries={chatMessagesData}
onUpdateChatData={updateChatData}>
</ChatLog>
</main>
</div>
);
Expand Down
9 changes: 4 additions & 5 deletions src/App.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('Wave 03: clicking like button and rendering App', () => {
fireEvent.click(buttons[10])

// Assert
const countScreen = screen.getByText(/3 ❤️s/)
const countScreen = screen.getByText(/3 💙s/)
expect(countScreen).not.toBeNull()
})

Expand All @@ -26,10 +26,9 @@ describe('Wave 03: clicking like button and rendering App', () => {
const lastButton = buttons[buttons.length - 1]

// Act-Assert

// click the first button
fireEvent.click(firstButton)
expect(firstButton.innerHTML).toEqual('❤️')
expect(firstButton.innerHTML).toEqual('💙')

// check that all other buttons haven't changed
for (let i = 1; i < buttons.length; i++) {
Expand All @@ -40,13 +39,13 @@ describe('Wave 03: clicking like button and rendering App', () => {
fireEvent.click(firstButton)
expect(firstButton.innerHTML).toEqual('🤍')
fireEvent.click(firstButton)
expect(firstButton.innerHTML).toEqual('❤️')
expect(firstButton.innerHTML).toEqual('💙')
fireEvent.click(firstButton)
expect(firstButton.innerHTML).toEqual('🤍')

// click the last button a couple times
fireEvent.click(lastButton)
expect(lastButton.innerHTML).toEqual('❤️')
expect(lastButton.innerHTML).toEqual('💙')
fireEvent.click(lastButton)
expect(lastButton.innerHTML).toEqual('🤍')
})
Expand Down
2 changes: 1 addition & 1 deletion src/components/ChatEntry.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,4 @@ button {

.chat-entry.remote .entry-bubble:hover::before {
background-color: #a9f6f6;
}
}
42 changes: 36 additions & 6 deletions src/components/ChatEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,51 @@ import React from 'react';
import './ChatEntry.css';
import PropTypes from 'prop-types';

function getDifferenceInYears(timeStamp) {
const today = new Date();
const year = today.getFullYear();

const pastDay = new Date(timeStamp);
const pastYear = pastDay.getFullYear();

const difference = year - pastYear;
return difference;
}
Comment on lines +5 to +14
Copy link

Choose a reason for hiding this comment

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

Nice job implementing your own solution for determining the time elapsed since a message was sent. If you check the components directory, you'll see TimeStamp component for you to use.

You can see how we implemented finding the time elapsed


const ChatEntry = (props) => {
const timeInYears = getDifferenceInYears(props.timeStamp) + ' years ago';

const onLikeButtonClick = () => {
const updatedMessage = {
id: props.id,
sender: props.sender,
body: props.body,
timeStamp: props.timeStamp,
liked: !props.liked
};

props.onUpdateChatData(updatedMessage);
};
Comment on lines +19 to +29
Copy link

Choose a reason for hiding this comment

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

If you refactor onUpdateChatData as I suggested in App.js, then your onLikeButtonClick() method only needs to modify the liked property of of a ChatEntry. If you follow this approach, you'd also need to introduce useState to keep track of a ChatEntry's like state.

The benefit of only changing the liked property is that when a user clicks on a like, they really are only changing liked. Therefore, we shouldn't let the onLikeButtonClick event handler change id, sender, body, timestamp (which is more editing power than this method really needs).

Reworking a solution so only liked is being affected will require you to do more than change a line or two of code, but it would best reflect what actually needs to happen. We should always prefer to only change the properties/data that need to be changed when we code.


return (
<div className="chat-entry local">
<h2 className="entry-name">Replace with name of sender</h2>
<div className={props.sender==='Vladimir'?'chat-entry local':'chat-entry remote'}>
<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">{timeInYears}</p>
Copy link

Choose a reason for hiding this comment

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

How would you refactor line 36 to use the custom TimeStamp component? What would you need to pass into the component as a prop?

<button onClick={onLikeButtonClick} className="like">{!props['liked']?'🤍':'💙'}</button>
</section>
</div>
);
};

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

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 @@
import React from 'react';
import './ChatLog.css';
import ChatEntry from './ChatEntry';
import PropTypes from 'prop-types';

const ChatLog = (props) => {
const messageComponents = props.entries.map((message) => {
return (
<ChatEntry
key={message['timeStamp']}
id={message['id']}
sender={message['sender']}
body={message['body']}
timeStamp={message['timeStamp']}
liked={message['liked']}
onUpdateChatData={props.onUpdateChatData}>
</ChatEntry>
);
});

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

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

export default ChatLog;
Copy link

Choose a reason for hiding this comment

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

👍 Nicely done