Skip to content

Курганов Артемий#36

Open
artemijkurganov wants to merge 2 commits intokontur-web-courses:masterfrom
artemijkurganov:master
Open

Курганов Артемий#36
artemijkurganov wants to merge 2 commits intokontur-web-courses:masterfrom
artemijkurganov:master

Conversation

@artemijkurganov
Copy link

No description provided.

Copy link

@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, то линтер почти всё подчёркивает красным( Рекомендую автофиксить все проблемы, перед тем, как пушить изменения

</head>
<body>
<p>Hi! I don't have React. Yet</p>
<div id="someId"></div>

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>,

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);

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) => {

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>) }

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, хотя по документации должно быть всё ок. Код запускается и работает*/}

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 = {

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

Comments