-
Notifications
You must be signed in to change notification settings - Fork 53
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
Курганов Артемий #36
base: master
Are you sure you want to change the base?
Курганов Артемий #36
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Круто, что вынес отдельные компонентики, следующий шаг — вынести их в отдельные файлы, так проще в коде ориентироваться, когда у тебя не один файл на 200 строк, а несколько по 20
А ещё, если открыть код в ide, то линтер почти всё подчёркивает красным( Рекомендую автофиксить все проблемы, перед тем, как пушить изменения
userProfile/index.html
Outdated
@@ -5,7 +5,7 @@ | |||
<title>React Interface</title> | |||
</head> | |||
<body> | |||
<p>Hi! I don't have React. Yet</p> | |||
<div id="someId"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Неплохой, конечно, айдишник, но если бы это был код в реальном проекте, я бы докопалась) Хотя бы универсальным root
его назвать, или что-то более конкретное, типа formPage
userProfile/src/index.tsx
Outdated
|
||
|
||
ReactDom.render( | ||
<Form></Form>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если у компонента или элемента нет детей, то можно сократить, и написать вот так: <Form/>
Аналогичные моменты есть и выше, с инпутами и селектом
userProfile/src/index.tsx
Outdated
let [isOpened, setOpenedState] = useState(false); | ||
|
||
let [name, setName] = useState(''); | ||
let [surname, setSurname] = useState(''); | ||
let [city, setCity] = useState(''); | ||
|
||
let [nameEmpty, setNameEmpty] = useState(false); | ||
let [surnameEmpty, setSurnameEmpty] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Эти штуки лучше объявить через const, линтер в моей IDE даже ругается на это) Они же действительно никогда не меняются в обычном джаваскриптовом смысле — если они меняются, то это значит, что компонент ререндерится == функция Form выполняется заново, и это уже новые объекты в памяти, а не старые
userProfile/src/index.tsx
Outdated
openModal(); | ||
}; | ||
|
||
const add_if_updated = (field_name: string, new_value: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Какой-то python_case проскочил, атата))
userProfile/src/index.tsx
Outdated
<Modal onClose={close}> | ||
<Modal.Header>Пользователь сохранен</Modal.Header> | ||
<Modal.Body> | ||
{ changeMessage.map(data => <p>{data}</p>) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Во время рендера модалки в консоли ошибки, что у элементов нет свойства key. Лучше всегда его указывать, когда вот так через map рендеришь компоненты
userProfile/src/index.tsx
Outdated
<div className='inputRow'> | ||
<Gapped gap={20}> | ||
<p className='title'>{title}</p> | ||
{/* Почему-то здесь ругается IDE, хотя по документации должно быть всё ок. Код запускается и работает*/} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это потому, что в селект можно передать items самых разных типов, и onValueChange работает как раз с тем же типом, что и у items. Да, ты передаёшь массив строк, но тайпскрипт всё равно не справляется с автовыводом типов, и ему надо помочь, явно его указав, вот так: <Select items={cities} ... />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Select<string> items={cities} ... />
Markdown съел дженерик 😄
userProfile/src/index.tsx
Outdated
) | ||
} | ||
|
||
type InputType = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Типам, которые описывают параметры компонентов, принято добавлять в конец Props. То есть это должен быть не InputType
, а InputProps
No description provided.