C24 Possum Ekaterina Belova Chat log by edbelova · Pull Request #25 · Ada-C24/react-chatlog

edbelova · 2025-12-16T08:01:40Z

No description provided.

…ponent Chat log to display Chat entry. Updated chat Entry to show the text from messages.json

…number displays at the top of the screen

…erColorPicker components; add corresponding styles

- Introduced functionality to choose font colors for local and remote senders. - Updated components: App, Header, ChatLog, ChatEntry, and SenderColorPicker to support color selection. - Removed App.css as it was no longer needed.

…orPicker components

Copilot

Pull request overview

This PR implements a fully functional chat application with interactive features including message liking and customizable sender colors. The implementation replaces placeholder content with a complete chat log system displaying messages between Vladimir and Estragon.

Key changes:

Added interactive color picker widgets allowing users to customize text colors for each sender
Implemented a heart counter displaying the total number of liked messages with toggle functionality
Refactored CSS by moving styles from App.css into component-specific stylesheets for better modularity

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/components/SenderColorPicker.jsx	New component providing color selection buttons for customizing sender text colors
src/components/SenderColorPicker.css	Styling for color picker including color utility classes and button layout
src/components/HeartCounter.jsx	New component displaying the total count of liked messages
src/components/HeartCounter.css	Widget styling including shared styles for multiple components
src/components/Header.jsx	New header component managing color state and composing HeartCounter and SenderColorPicker components
src/components/Header.css	Header layout and styling with fixed positioning
src/components/ChatLog.jsx	New component rendering the list of chat entries with PropTypes validation
src/components/ChatEntry.jsx	Implements chat message display with like functionality, timestamp rendering, and dynamic color styling
src/components/ChatEntry.css	Adds flexbox layout for entry attributes and like button styling
src/App.jsx	Main application logic including state management for messages, likes, and color preferences
src/App.css	Removes styles that were moved to component-specific CSS files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T08:06:57Z

+span {
+  display: inline-block
+}


The CSS selector 'span' is too broad and will affect all span elements globally in the application. This should be scoped to avoid unintended side effects on other components. Consider using a more specific selector like '.sender-color-picker span' to limit the scope to this component only.

Copilot · 2025-12-16T08:06:57Z

+.red {
+  color: #b22222
+}
+
+.orange {
+  color: #e6ac00
+}
+
+.yellow {
+  color: #e6e600
+}
+
+.green {
+  color: green
+}
+
+.blue {
+  color: blue
+}
+
+.purple {
+  color: purple
+}
+
+.black {


The color utility classes (red, orange, yellow, green, blue, purple, black) are defined globally without scoping. These classes will affect any element with these class names across the entire application. Consider either scoping them to this component (e.g., '.sender-color-picker .red') or moving them to a shared utility CSS file if they're intended to be used globally.

Suggested change

.red {

color: #b22222

}

.orange {

color: #e6ac00

}

.yellow {

color: #e6e600

}

.green {

color: green

}

.blue {

color: blue

}

.purple {

color: purple

}

.black {

.senderColorPicker .red {

color: #b22222

}

.senderColorPicker .orange {

color: #e6ac00

}

.senderColorPicker .yellow {

color: #e6e600

}

.senderColorPicker .green {

color: green

}

.senderColorPicker .blue {

color: blue

}

.senderColorPicker .purple {

color: purple

}

.senderColorPicker .black {

Copilot · 2025-12-16T08:06:58Z

+  color: black
+}
+
+.senderColorPicker span {


The selector '.senderColorPicker' uses camelCase but doesn't match the actual class name 'sender-color-picker' (kebab-case) used in the JSX component. This CSS rule will not apply to any elements. The selector should be '.sender-color-picker span' to match the component's class name.

Suggested change

.senderColorPicker span {

.sender-color-picker span {

Copilot · 2025-12-16T08:06:58Z

+  margin: 1em;
+}
+
+#App span {


The CSS selector '#App span' is too broad and will affect all span elements throughout the entire application. This could cause unintended styling side effects on other components. Consider using a more specific selector scoped to this component.

Suggested change

#App span {

#App #heartWidget span {

Copilot · 2025-12-16T08:06:58Z

+  const colorButtonClicked = (color) => {
+    chooseFontColor(senderName, color);
+    setColor(color);


The parameter name 'color' shadows the state variable 'color' defined on line 6. While functionally correct, this can reduce code clarity. Consider renaming the parameter to something like 'selectedColor' or 'newColor' to avoid the naming collision.

Suggested change

const colorButtonClicked = (color) => {

chooseFontColor(senderName, color);

setColor(color);

const colorButtonClicked = (selectedColor) => {

chooseFontColor(senderName, selectedColor);

setColor(selectedColor);

Copilot · 2025-12-16T08:06:59Z

+    })
+  ).isRequired,
+  localSender: PropTypes.string,
+  onToggleHeart: PropTypes.func,


The PropTypes definition is incomplete and doesn't match the component's actual props. The component receives 'fontColorForLocalSender' and 'fontColorForRemoteSender' props (lines 5, 18-19) but these are not defined in PropTypes. Add these missing prop type definitions to ensure proper type checking.

Suggested change

onToggleHeart: PropTypes.func,

onToggleHeart: PropTypes.func,

fontColorForLocalSender: PropTypes.string,

fontColorForRemoteSender: PropTypes.string,

Copilot · 2025-12-16T08:07:00Z

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


The CSS rules defined here (.widget, #heartWidget, span) appear to be component-specific styles but are placed in HeartCounter.css. Since these styles are shared across multiple components (HeartCounter and potentially SenderColorPicker use .widget class), consider moving these to a shared stylesheet or the main App.css file to avoid duplication and maintain consistency.

Copilot · 2025-12-16T08:07:00Z

+  const isLikedButtonClicked = () => {
+    props.onToggleHeart(props.id);
+  };


The function name 'isLikedButtonClicked' uses a naming pattern that describes state rather than an action. Consider renaming to 'handleLikeClick' or 'handleToggleLike' to better indicate the action being performed and follow React naming conventions for event handlers.

Copilot · 2025-12-16T08:07:00Z

+  timeStamp: PropTypes.string.isRequired,
+  localSender: PropTypes.string,
+  isLiked: PropTypes.bool,
+  onToggleHeart: PropTypes.func,


The PropTypes definition is incomplete and doesn't match the component's actual props. The component receives 'id', 'fontColorForLocalSender', and 'fontColorForRemoteSender' props (lines 10, 13) but these are not defined in PropTypes. Add these missing prop type definitions to ensure proper type checking.

Suggested change

onToggleHeart: PropTypes.func,

onToggleHeart: PropTypes.func,

id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,

fontColorForLocalSender: PropTypes.string.isRequired,

fontColorForRemoteSender: PropTypes.string.isRequired,

Copilot · 2025-12-16T08:07:00Z

+const ChatEntry = (props) => {
+  const isLocal = props.sender === props.localSender;
+  const entryClass = isLocal ? 'local' : 'remote';
+  const isLiked = props.isLiked ? '❤️' : '🤍';


The variable name 'isLiked' is misleading as it stores the emoji string to display, not a boolean value. Consider renaming it to 'likeIcon' or 'heartIcon' to better reflect what it contains.

yangashley · 2025-12-18T20:10:55Z

+  const [fontColorForLocalSender, setFontColorForLocalSender] = useState('black');
+  const [fontColorForRemoteSender, setFontColorForRemoteSender] = useState('black');


❗️👀 The same concept (“what color does each sender use?”) is being represented in 3 components (App, Header, and SenderColorPicker), which is a strong signal that state placement needs to be cleaned up. There is no longer a single source of truth, but 3 sources of truth!

Again, my eyebrows were raised when I saw another place where there is state that is related to color.

Now that I see how the 3 components work together, I see that SenderColorPicker only needs to trigger a change. It does not need to “own” or persist the color itself and Header doesn't need to either.

When I think about App:

It should own the source of truth for sender colors (this is where the state for colors should live)

It should decides how many senders exist and what their colors are

It passes the current colors down

It should have a function for updating colors and passes it down

Header should:
- Have no color state (remove it)

Be purely presentational

Receives colors as props

Passes the event handler callback related to colors down to the picker

SenderColorPicker should:
- Have no color state (remove it)

Have no business logic because it only displays the UI

Notify when a color is chosen

🧠 We should identify all the components need to know about color and identify their closest common ancestor.

We should also really consider what each component should know and it should be responsible for. This is why we put so much emphasis on pre-planning your components during the activity related to task-list-front-end (where the first wave was looking at a mockup and identifying what components you needed and how data should flow between the components).

For future projects it will be critical to do this kind of planning for your components before you start writing code so you don't create many different sources of truth by introducing redundant pieces of state.

yangashley · 2025-12-22T14:40:39Z

+const ChatEntry = (props) => {
+  const isLocal = props.sender === props.localSender;


Destructuring props can be a matter of preference. I prefer to destructure props so that I don't need to type props. every time I want to get the attribute from the object.

Destructuring props also has some benefits:

you know every possible prop right away.

you know when a prop is being unused, in order to remove it.

knowing which props are being used will also remind you what you need to validate (I see a review comment from copilot for line 39 below that calls out props that weren't validated with propTypes).

Ultimately, you'll adapt what you do depending on what your team does.

Suggested change

const ChatEntry = (props) => {

const isLocal = props.sender === props.localSender;

const ChatEntry = ( {sender, localSender} ) => {

const isLocal = sender === localSender;

yangashley · 2025-12-22T14:42:46Z

+      <h2 className="entry-name">{props.sender}</h2>
      <section className="entry-bubble">


While not strictly required by the HTML specification, it is a strong best practice and highly recommended that section elements should have a heading element inside them.

You could put this h2 on line 17 inside the section on line 18 to fulfill this specification.

yangashley · 2025-12-22T14:43:06Z

+        <p className={`entry-body ${fontColor}`}>{props.body}</p>
+        <div className="entry-attributes">
+          <p className="entry-time">
+            <TimeStamp time={props.timeStamp} />


Nice job using the supplied TimeStamp component here 👍

yangashley · 2025-12-22T14:46:22Z

+import ChatEntry from './ChatEntry';
+import PropTypes from 'prop-types';
+
+const ChatLog = ({entries, localSender, onToggleHeart, fontColorForLocalSender, fontColorForRemoteSender}) => {


Nice work destructuring your props here 👍

yangashley · 2025-12-22T14:52:54Z

+  const [localColor, setLocalColor] = useState('white');
+  const [remoteColor, setRemoteColor] = useState('white');


Instead of using 2 pieces of state to track colors for the local and remote sender, we can use one piece with an object for the initial state.

Wrt to state, it's beneficial to evaluate what pieces of state do we really need and how can we simplify state so that our project won't be overly complicated.

Suggested change

const [localColor, setLocalColor] = useState('white');

const [remoteColor, setRemoteColor] = useState('white');

const [senderColor, setSenderColor] = useState(

{

local: 'white',

remote: 'white'

}

);

yangashley · 2025-12-22T14:54:47Z

+  return (
+    <header>
+      <h1>
+        Chat between <span className={localColor}>Vladimir</span> and <span className={remoteColor}>Estragon</span>


Can you think of a way to dynamically get the senders' names without hardcoding "Vladimir" and "Estragon" here?

yangashley · 2025-12-22T15:07:40Z

+
+const SenderColorPicker = ({ senderName, chooseFontColor }) => {
+
+  const [color, setColor] = useState('black');


👀🚨 Seeing another piece of state related to color in SenderColorPicker made me raise my eyebrows about whether this is redundant or not. As you work with state in React.js more you'll get a better feel for when you need state or not.

In React, a piece of state should live in exactly one place: the lowest common ancestor that owns that data and needs to coordinate it.

When you are going to introduce state, ask yourself two questions for each piece of state:

Who needs to know this value?

Who is responsible for changing it?

The state in SenderColorPicker is redundant because SenderColorPicker currently stores its own color and also notifies the parent (Header) about the color.

This creates two problems because the SenderColorPicker is remembering something the parent already knows, which could create problems:

Duplicated state (picker color vs header color)

Risk of desynchronization (they could drift apart)

State related to color should be removed from SenderColorPicker here.

yangashley · 2025-12-22T15:12:53Z

+    <div className="widget sender-color-picker">
+      <h3 className={`senderLabel ${color}`}>{senderName}'s color:</h3>
+      <span className={senderName.toLowerCase()}>
+        <button onClick={() => colorButtonClicked('red')}>🔴</button>


I'd prefer introducing an object that stores all the different colors and then iterating over the object to populate the values for each button element.

That would make the component a little more concise and easier to maintain.

Maybe something like:

const BUTTON_COLORS = { red: '🔴', orange: '🟠' yellow: '🟡' green: '🟢' blue: '🔵' purple: '🟣', };

yangashley · 2025-12-22T15:28:46Z

+    setEntries(entries => {
+      return entries.map(entry => {


Nice job using the callback-style for updating the state.

edbelova added 12 commits December 10, 2025 15:45

Updated component Add to display Chat log and Chat entry. Updated com…

d91b1a0

…ponent Chat log to display Chat entry. Updated chat Entry to show the text from messages.json

Implement Wave 3: like functionality in chat entries and total count …

202327b

…number displays at the top of the screen

Implement Optional Color Choice: added Header, HeartCounter, and Send…

d8330d5

…erColorPicker components; add corresponding styles

Implement dynamic font color change in header in Header and SenderCol…

c12ceb4

…orPicker components

Update header background color for improved visibility

323c4e0

Wrap entry time and like button in a flex container for better layout

14b8af9

Fix button padding and add styling for like button

5d6daa3

Update propTypes for ChatEntry to make isLiked optional

d5138f8

Make localSender and onToggleHeart propTypes optional in ChatLog

600f8dd

Make localSender propType optional in ChatEntry

c7445dc

Make onToggleHeart propType optional in ChatEntry

deddd0e

Copilot AI review requested due to automatic review settings December 16, 2025 08:01

Copilot started reviewing on behalf of edbelova December 16, 2025 08:02 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

yangashley reviewed Dec 22, 2025

View reviewed changes

-  onToggleHeart: PropTypes.func,
+  onToggleHeart: PropTypes.func,
+  id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired,
+  fontColorForLocalSender: PropTypes.string.isRequired,
+  fontColorForRemoteSender: PropTypes.string.isRequired,

		const [fontColorForLocalSender, setFontColorForLocalSender] = useState('black');
		const [fontColorForRemoteSender, setFontColorForRemoteSender] = useState('black');

		const ChatEntry = (props) => {
		const isLocal = props.sender === props.localSender;

		<h2 className="entry-name">{props.sender}</h2>
		<section className="entry-bubble">

		const [localColor, setLocalColor] = useState('white');
		const [remoteColor, setRemoteColor] = useState('white');

-  const [localColor, setLocalColor] = useState('white');
-  const [remoteColor, setRemoteColor] = useState('white');
+  const [senderColor, setSenderColor] = useState(
+    {
+       local: 'white',
+       remote: 'white'
+    }
+  );


		const SenderColorPicker = ({ senderName, chooseFontColor }) => {

		const [color, setColor] = useState('black');

Conversation

edbelova commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants