Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

artemijkurganov
Copy link

No description provided.

Copy link
Collaborator

@Ollisteka Ollisteka left a comment

Choose a reason for hiding this comment

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

Круто, что вынес отдельные компонентики, следующий шаг — вынести их в отдельные файлы, так проще в коде ориентироваться, когда у тебя не один файл на 200 строк, а несколько по 20

А ещё, если открыть код в ide, то линтер почти всё подчёркивает красным( Рекомендую автофиксить все проблемы, перед тем, как пушить изменения

@@ -5,7 +5,7 @@
<title>React Interface</title>
</head>
<body>
<p>Hi! I don't have React. Yet</p>
<div id="someId"></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Неплохой, конечно, айдишник, но если бы это был код в реальном проекте, я бы докопалась) Хотя бы универсальным root его назвать, или что-то более конкретное, типа formPage



ReactDom.render(
<Form></Form>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если у компонента или элемента нет детей, то можно сократить, и написать вот так: <Form/>
Аналогичные моменты есть и выше, с инпутами и селектом

Comment on lines 71 to 78
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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Эти штуки лучше объявить через const, линтер в моей IDE даже ругается на это) Они же действительно никогда не меняются в обычном джаваскриптовом смысле — если они меняются, то это значит, что компонент ререндерится == функция Form выполняется заново, и это уже новые объекты в памяти, а не старые

openModal();
};

const add_if_updated = (field_name: string, new_value: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Какой-то python_case проскочил, атата))

<Modal onClose={close}>
<Modal.Header>Пользователь сохранен</Modal.Header>
<Modal.Body>
{ changeMessage.map(data => <p>{data}</p>) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Во время рендера модалки в консоли ошибки, что у элементов нет свойства key. Лучше всегда его указывать, когда вот так через map рендеришь компоненты

<div className='inputRow'>
<Gapped gap={20}>
<p className='title'>{title}</p>
{/* Почему-то здесь ругается IDE, хотя по документации должно быть всё ок. Код запускается и работает*/}
Copy link
Collaborator

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} ... />

Copy link
Author

@artemijkurganov artemijkurganov Oct 19, 2022

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 съел дженерик 😄

)
}

type InputType = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Типам, которые описывают параметры компонентов, принято добавлять в конец Props. То есть это должен быть не InputType, а InputProps

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