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.

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

Say Zoisite #101

Open
wants to merge 2 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
51 changes: 47 additions & 4 deletions src/App.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,59 @@
import React from 'react';
import React, { useState } from 'react';
import './App.css';
import ChatLog from './components/ChatLog';
import chatMessages from './data/messages.json';
import ColorChoice from './components/ColorChoice';

const App = () => {
const [messages, setMessages] = useState(() => {
const participants = Array.from(new Set(chatMessages.map((message) => message.sender)));
const updatedMessages = chatMessages.map((message) => ({
...message,
type: participants.indexOf(message.sender) === 0 ? 'local' : 'remote',

Choose a reason for hiding this comment

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

Neat way to set the initial state and determine local vs. remote so you can just read and use that value at the ChatEntry level.

}));
return updatedMessages;
});

const [color, setColor] = useState('#000000');
const [heartCount, setHeartCount] = useState(0);

const toggleLike = (id) => {
setMessages((prevMessages) => {
return prevMessages.map((message) => {
if (message.id === id) {
const updatedMessage = { ...message, liked: !message.liked };
if (updatedMessage.liked) {
setHeartCount((prevCount) => prevCount + 1);
} else {
setHeartCount((prevCount) => prevCount - 1);
Comment on lines +24 to +28

Choose a reason for hiding this comment

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

Our state messages contains info on whether a message is liked or not, which means we could derive our heart count from the data by looping over messages and counting up how many have a liked value of True. This would let us remove the heartCount state that we need to manually keep in sync with the changes to messages. What changes would we need to make for that to work?

}
return updatedMessage;
}
return message;
});
});
};

const setParticipantColor = (color) => {
setColor(color);
};

const participants = Array.from(new Set(chatMessages.map((message) => message.sender)));

const headerTitle = participants.length === 1 ? participants[0] : `${participants[0]} and ${participants[1]}`;

return (
<div id="App">
<header>
<h1>Application title</h1>
<h1>Say What</h1>
<h3>{headerTitle}</h3>

Choose a reason for hiding this comment

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

In the MDN docs it's highly suggested to not skip headings:

A common navigation technique for users of screen reading software is jumping from heading to quickly determine the content of the page. Because of this, it is important to not skip one or more heading levels. Doing so may create confusion, as the person navigating this way may be left wondering where the missing heading is.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#navigation

Here the suggestion would be to use h2 and style it as needed to match the smaller font size desired.

<div>

Choose a reason for hiding this comment

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

Is this div necessary for styling? I'm wondering if it could be removed or changed into a semantic element.

<h2>{heartCount} ❤️s</h2>
</div>
<ColorChoice setColorCallback={setParticipantColor} />
</header>
<main>
{/* Wave 01: Render one ChatEntry component
Wave 02: Render ChatLog component */}
<ChatLog entries={messages} onLike={toggleLike} color={color} />
</main>
</div>
);
Expand Down
97 changes: 29 additions & 68 deletions src/components/ChatEntry.css
Original file line number Diff line number Diff line change
@@ -1,59 +1,3 @@
button {

Choose a reason for hiding this comment

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

I'm seeing a number of changes in the CSS files, did you run into any issues with applying the styles?

background: none;
color: inherit;
border: none;
padding: 10px;
font: inherit;
cursor: pointer;
outline: inherit;
}

.chat-entry {
margin: 1rem;
}

.chat-entry:last-child {
margin-bottom: 0;
}

.chat-entry .entry-bubble {
background-color: #ffffe0;
border-radius: 30px;
max-width: 50rem;
min-width: 10rem;
padding: 1rem 1rem 0.1rem 1rem;
position: relative;
width: fit-content;
}

.chat-entry .entry-bubble:hover {
background-color: #fefea2;
}

.chat-entry .entry-name {
font-size: medium;
margin-bottom: 0.5rem;
}

.chat-entry .entry-time {
color: #bbb;
font-size: x-small;
margin-bottom: 0.1rem;
margin-right: 0.5rem;
}

/* Chat bubble arrow styling */
.chat-entry .entry-bubble::before {
content: '';
height: 22px;
width: 44px;
clip-path: polygon(100% 0, 0 0, 50% 100%);

position: absolute;
top: 0;
}

/* "local" messages are shown on the left side */
.chat-entry.local {
text-align: left;
}
Expand All @@ -62,39 +6,56 @@ button {
text-align: right;
}

.chat-entry.local .entry-bubble {
background-color: #ffffe0;
border-radius: 20px;
display: inline-block;
padding: 8px 12px;
position: relative;
}

.chat-entry.local .entry-bubble::before {
background-color: #ffffe0;
content: "";
height: 20px;
position: absolute;
bottom: -10px;
left: -18px;
transform: rotate(45deg);
width: 20px;
}

.chat-entry.local .entry-bubble:hover::before {
background-color: #fefea2;
}

/* "remote" messages are shown on the right side, in blue */
.chat-entry.remote {
text-align: right;
}

.chat-entry.remote .entry-bubble {
background-color: #e0ffff;
margin-left: auto;
margin-right: 0;
}

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

.chat-entry.remote .entry-time {
text-align: left;
}

.chat-entry.remote .entry-bubble {
background-color: #e0ffff;
border-radius: 20px;
display: inline-block;
padding: 8px 12px;
position: relative;
}

.chat-entry.remote .entry-bubble::before {
background-color: #e0ffff;
content: "";
height: 20px;
position: absolute;
bottom: -10px;
right: -18px;
transform: rotate(45deg);
width: 20px;
}

.chat-entry.remote .entry-bubble:hover::before {
background-color: #a9f6f6;
}
}
30 changes: 22 additions & 8 deletions src/components/ChatEntry.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,36 @@
import React from 'react';
import './ChatEntry.css';
import PropTypes from 'prop-types';
import TimeStamp from './TimeStamp';

const ChatEntry = (props) => {
const ChatEntry = ({id, sender, body, timeStamp, liked, type, color, onLike}) => {
const entryClass = type === 'local' ? 'local' : 'remote';
const bubbleClass = type === 'local' ? 'local' : 'remote';
Comment on lines +7 to +8

Choose a reason for hiding this comment

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

If type is a string that contains either "local" or "remote", could we remove these two variables and use type directly in their place?

return (
<div 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>
<div className={`chat-entry ${entryClass}`}>
<h2 className={`entry-name ${bubbleClass}`}style={{ color: color }}>
Copy link

@kelsey-steven-ada kelsey-steven-ada Jun 27, 2023

Choose a reason for hiding this comment

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

We should avoid inline styles to separate the layers of our web app and keep everything organized. Most modern browsers support CSS custom properties, which we can use to achieve a similar effect. (Info on CSS Custom Properties support: https://caniuse.com/css-variables).

What we could do is declare a css variable that is set to the default color of black, and use that variable in a css rule that applies to the element we want to see the change on. Then when a new color is selected, we would update that CSS variable.

// In ChatEntry.css

// Create a CSS custom property (variable) that's globally accessible
// we could use an element other than root, but we would need to look up 
// the --curr-color var on that element when we want to change it later.
:root {
  --curr-color: black;
}

// Declare a rule that targets the sender names and uses the CSS variable
.chat-entry h2 {
  color: var(--curr-color);
}
// In ColorChoice.js

const handleColorChange = (e) => {
    // Update the state for the color picker
    const selectedColor = e.target.value;
    setColor(selectedColor);

    // Update the CSS variable to the chosen color
    // This is a little hacky, we would probably still have a piece of react state in App.js that we update here
    // but in App.js, somewhere before returning the JSX we would call the line below to ensure the CSS
    // var matches the React state, ensuring the React State is still our source of truth for what the user sees
    document.documentElement.style.setProperty("--curr-color", selectedColor);
};

Choose a reason for hiding this comment

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

Here's a link to an article that dives into what I'm talking about above: https://spacejelly.dev/posts/how-to-create-css-custom-properties-that-dynamically-update-with-react-javascript/

{sender}
</h2>
<section className='entry-bubble'>
<p>{body}</p>
<TimeStamp className="entry-time" time={timeStamp} />
<button className="like" onClick={() => onLike(id)}>
{liked ? '❤️' : '🤍'}
</button>
</section>
</div>
);
};

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

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

const ChatLog = ({ entries, color, onLike }) => {
const chatComponents = entries.map((entry, i) => (
<ChatEntry
key={i}
id={i}
{...entry}
color={color}
onLike={onLike}
/>
Comment on lines +6 to +13

Choose a reason for hiding this comment

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

The best practice is to only default to using an index for key if there is no unique id provided for the elements. In this case, our data has a unique id for every message, so we should use that value as both the key and id props here. Looking forward, when reading and displaying data from a database, those records typically have a unique ID field we should prefer to use.

));

return <div className="chat-log">{chatComponents}</div>;
};

ChatLog.propTypes = {
entries: PropTypes.arrayOf(PropTypes.object).isRequired,
color: PropTypes.string.isRequired,
onLike: PropTypes.func.isRequired,
};

export default ChatLog;
24 changes: 24 additions & 0 deletions src/components/ColorChoice.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React, { useState } from 'react';

const ColorChoice = (props) => {
const [color, setColor] = useState('#000000');

const handleColorChange = (e) => {
const selectedColor = e.target.value;
setColor(selectedColor);
props.setColorCallback(selectedColor);
};

return (
<div className="color-choice">
<h3>Select Color:</h3>
<input
type="color"
value={color}
onChange={handleColorChange}
/>
Comment on lines +15 to +19

Choose a reason for hiding this comment

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

Awesome exploration of input types! If we wanted to take this further, what would we need to change about the project if we wanted the user to be able to select different colors for each participant in the chat?

</div>
);
};

export default ColorChoice;