Say Zoisite #101
Say Zoisite #101
Conversation
| 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', |
There was a problem hiding this comment.
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.
| const updatedMessage = { ...message, liked: !message.liked }; | ||
| if (updatedMessage.liked) { | ||
| setHeartCount((prevCount) => prevCount + 1); | ||
| } else { | ||
| setHeartCount((prevCount) => prevCount - 1); |
There was a problem hiding this comment.
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?
| <h1>Application title</h1> | ||
| <h1>Say What</h1> | ||
| <h3>{headerTitle}</h3> | ||
| <div> |
There was a problem hiding this comment.
Is this div necessary for styling? I'm wondering if it could be removed or changed into a semantic element.
| return ( | ||
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Say What</h1> | ||
| <h3>{headerTitle}</h3> |
There was a problem hiding this comment.
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.
| const entryClass = type === 'local' ? 'local' : 'remote'; | ||
| const bubbleClass = type === 'local' ? 'local' : 'remote'; |
There was a problem hiding this comment.
If type is a string that contains either "local" or "remote", could we remove these two variables and use type directly in their place?
| <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 }}> |
There was a problem hiding this comment.
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);
};There was a problem hiding this comment.
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/
| const chatComponents = entries.map((entry, i) => ( | ||
| <ChatEntry | ||
| key={i} | ||
| id={i} | ||
| {...entry} | ||
| color={color} | ||
| onLike={onLike} | ||
| /> |
There was a problem hiding this comment.
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.
| <input | ||
| type="color" | ||
| value={color} | ||
| onChange={handleColorChange} | ||
| /> |
There was a problem hiding this comment.
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?
| @@ -1,59 +1,3 @@ | |||
| button { | |||
There was a problem hiding this comment.
I'm seeing a number of changes in the CSS files, did you run into any issues with applying the styles?
No description provided.