Skip to content

Develop#207

Open
nastia7408 wants to merge 3 commits into
mate-academy:masterfrom
nastia7408:develop
Open

Develop#207
nastia7408 wants to merge 3 commits into
mate-academy:masterfrom
nastia7408:develop

Conversation

@nastia7408
Copy link
Copy Markdown

No description provided.

Nastia Mazur added 2 commits May 1, 2026 22:22
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Your solution demonstrates strong progress—you've successfully implemented all the core requirements including room creation, renaming, joining, deletion, message display with author and timestamps, localStorage persistence, and the Node.js/Express/Socket.io backend. Important fix needed: In client/src/pages/AuthPage.jsx (around line 97), the submit button uses type="button" instead of type="submit". This prevents the form's onSubmit handler from firing, meaning users cannot actually register or login. This one small change will make the entire authentication flow work. Minor improvement: Your ESLint config targets **/*.{ts,tsx} but all your source files use .jsx extension. Update the pattern to **/*.{js,jsx} to avoid configuration warnings. The server-side implementation, message display logic, and room management features are all well-implemented. Once you fix the button type issue, the core user flow will be complete. Keep up the excellent work.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread client/eslint.config.js

export default defineConfig([
globalIgnores(['dist']),
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ESLint config extends tseslint.configs.recommended and targets **/*.{ts,tsx} files, but the actual source files use .jsx extension (JavaScript). Either rename source files to .tsx/.ts or update the files pattern to **/*.{js,jsx} to match the actual codebase.

setFormData({ ...formData, password: e.target.value })
}
required
/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Button type is 'button' which prevents form submission. This button needs to be type="submit" so the form's onSubmit={handleSubmit} on line 57 is triggered. Without this fix, users cannot register or login at all.

Comment thread client/src/App.scss
flex-shrink: 0;
width: 60px;
border-radius: 50%;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The submit button has type="button" which does NOT trigger the form's onSubmit handler. This means users cannot submit the registration/login form by clicking the button. Change this to type="submit" so the form submission works correctly.

if (userNumber.trim() !== '') {
chatData.number = userNumber;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The dependency array includes setNewName which is a state setter function from useState - this should not be included in the dependency array as it can cause unnecessary re-runs or lint warnings. Only chat.id, chat.name, and prevvChatId are actual dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants