Develop #2921
Develop #2921
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! There are a few critical issues that need to be addressed before we can approve the pull request. Specifically, the typo in the function name handlUser should be corrected to handleUser for clarity and consistency. Additionally, the useState hooks for hasUserError and hasTitleError should be initialized with boolean values instead of strings, as they are meant to track error states. Once these changes are made, your code will be in great shape! Keep up the great work! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/App.tsx
Outdated
|
|
||
| export const App = () => { | ||
| const [title, setTitle] = useState(''); | ||
| const [hasTitleError, setHasTitleError] = useState(''); |
There was a problem hiding this comment.
The useState hook for hasTitleError should be initialized with a boolean value (e.g., false) instead of an empty string, as it is used to track the presence of an error.
src/App.tsx
Outdated
| const [hasTitleError, setHasTitleError] = useState(''); | ||
|
|
||
| const [selectedUserId, setSelectedUserId] = useState(0); | ||
| const [hasUserError, setHasUserError] = useState(''); |
There was a problem hiding this comment.
The useState hook for hasUserError should be initialized with a boolean value (e.g., false) instead of an empty string, as it is used to track the presence of an error.
src/App.tsx
Outdated
| setHasTitleError(''); | ||
| }; | ||
|
|
||
| const handlUser = (event: React.ChangeEvent<HTMLSelectElement>) => { |
There was a problem hiding this comment.
There is a typo in the function name handlUser. It should be handleUser to maintain consistency and clarity in naming conventions.
There was a problem hiding this comment.
Review Summary
Great job on addressing the previous issues and making improvements to your code! 🎉 Your SCSS file is well-structured, and the use of BEM naming conventions is commendable. The App.tsx file is mostly well-implemented, but please correct the import paths for getHigherId and getUserById functions to ensure the application functions correctly. Similarly, verify the import paths for the Todo and User types across your components to maintain proper type checking and avoid redundancy.
You've made excellent progress by correcting the function name and initializing the useState hooks appropriately. Keep up the good work, and make sure to address these minor issues in the future to enhance your code's functionality and maintainability. This decision is made to help you move forward with your learning journey. Keep improving and learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| import usersFromServer from './api/users'; | ||
| import todosFromServer from './api/todos'; | ||
| import { TodoList } from './components/TodoList'; | ||
| import { getHigherId } from './serveses/todos'; |
There was a problem hiding this comment.
The import path './serveses/todos' seems to be incorrect. Please verify the correct path for the getHigherId function.
| import todosFromServer from './api/todos'; | ||
| import { TodoList } from './components/TodoList'; | ||
| import { getHigherId } from './serveses/todos'; | ||
| import { getUserById } from './serveses/user'; |
There was a problem hiding this comment.
The import path './serveses/user' seems to be incorrect. Please verify the correct path for the getUserById function.
| import classNames from 'classnames'; | ||
| import React from 'react'; | ||
| import { UserInfo } from '../UserInfo'; | ||
| import { Todo } from '../../serveses/types'; |
There was a problem hiding this comment.
The import path '../../serveses/types' seems to be incorrect. Please verify the correct path for the Todo type.
| export const TodoList = () => {}; | ||
| import React from 'react'; | ||
| import { TodoInfo } from '../TodoInfo'; | ||
| import { Todo } from '../../serveses/types'; |
There was a problem hiding this comment.
The import path '../../serveses/types' seems to be incorrect. Please verify the correct path for the Todo type.
| @@ -1 +1,14 @@ | |||
| export const UserInfo = () => {}; | |||
| import React from 'react'; | |||
| import { User } from '../../serveses/types'; | |||
There was a problem hiding this comment.
The import path '../../serveses/types' seems to be incorrect. Please verify the correct path for the User type.
| name: string; | ||
| username: string; | ||
| email: string; | ||
| }; |
There was a problem hiding this comment.
The User type is redefined here, which is unnecessary. Consider importing the User type from types.ts to maintain consistency and avoid redundancy.
No description provided.