Sprint 2 #36
Sprint 2 #36
Conversation
There was a problem hiding this comment.
Здравствуйте. (Нужно развернуть общий комментарий ↓)
Работа проделана огромная
Что сделано отлично:
Readmeхорошо оформлен- Проект задеплоен и отлично работает
- Отлично, что не забыли про
.gitignore - Отлично, что ограничиваете версию
node - Отлично, что нет
EOFошибок в гите - Хорошая структура папок и файлов
- Добавлен
ESLintиStylelint - Отлично, что цвета вынесены в
css-константы - Отлично, что не забываете про
altв тегахimg - Отлично, что не забываете про тег
mainдля главной части страницы - Отлично, что моковые данные вынесены в отдельные файлы
Что нужно исправить:
- В теории давался еще метод
_removeEvents, который правильно удаляет обработчики перед размонтированием блока. Нужно его добавить вBlock. - По заданию 1го спринта нужно давать инпутам конкретные атрибуты
name, чтобы в 3м спринте не менять ничего при подключении кApi. Скрин из задания https://disk.yandex.ru/i/lSPqeGrAa20ayQ
Что можно улучшить:
- Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
Исправьте, пожалуйста, недочеты и работа будет принята. Пожалуйста, проверьте работоспособность проекта и наличие возможных ошибок в консоли браузера (кнопка F12) перед отправкой на ревью.
Напоминаю, что работа может быть принята только после исправления всех критических замечаний Нужно исправить.
Комментарии Можно лучше не обязательны к исправлению прямо сейчас, это рекомендации
Удачного рефакторинга кода.
| this._addEvents(); | ||
| } | ||
|
|
||
| _addEvents() { |
There was a problem hiding this comment.
Нужно исправить
В теории давался еще метод _removeEvents, который правильно удаляет обработчики перед размонтированием блока. Нужно его добавить в Block.
любой обработчик события должен навешиваться только через пропсы экземпляра класса, который наследуется от Block. В теории показано было, что в пропсах передается объект events со всеми нужными событиями, а в родительском классе Block они навешиваются через _addEvents и удаляются через _removeEvents. Посмотрите реализацию Block в теории
src/utils/Http.ts
Outdated
| } | ||
|
|
||
| class HTTPTransport { | ||
| get = (url:string, options:IOptionsRequest = {method: METHODS.GET},params:object) => { |
There was a problem hiding this comment.
Можно лучше
Если типизация повторяется в каждом методе, то нужно типизировать сам метод, а не дублировать типизацию аргументов. Код станет чище
// создаем тип метода
type HTTPMethod = (url: string, options?: IOptionsRequest) => Promise<unknown>
export class HTTPTransport {
// используем тип и удаляем дублирование в аргументах
get: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.GET}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
put: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.PUT}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
post: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.POST}, options.timeout)
)
// используем тип и удаляем дублирование в аргументах
delete: HTTPMethod = (url, options = {}) => (
this.request(url, {...options, method: METHODS.DELETE}, options.timeout)
)There was a problem hiding this comment.
Спасибо, очень удобная типизация методов
| classes="input__value ${error ? "input__value-error" : ""}" | ||
| value='${value}' | ||
| placeholder=" " | ||
| name="input" |
There was a problem hiding this comment.
Нужно исправить
По заданию 1го спринта нужно давать инпутам конкретные атрибуты name, чтобы в 3м спринте не менять ничего при подключении к Api. Скрин из задания https://disk.yandex.ru/i/lSPqeGrAa20ayQ
|
Тоже поздравляю, но PR нужно делать в свой репозиторий, а не в общий, иначе всем студентам будут уведомления приходить |
No description provided.