-
Notifications
You must be signed in to change notification settings - Fork 1
Надо подкачаться #12
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
Надо подкачаться #12
Conversation
js/alert-error.js
Outdated
| } | ||
| }; | ||
|
|
||
| function onDocumentKeydown (evt) { |
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.
Есть ли необходимость в function? Можно использовать стрелочную функцию?
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.
исправил
js/alert-success.js
Outdated
| } | ||
| }; | ||
|
|
||
| function onDocumentKeydown (evt) { |
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.
Функции в модулях повторяются - может объединить в один модуль и переиспользовать функции?
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.
объединил в модуле utils и импортировал
| @@ -0,0 +1,31 @@ | |||
| const BASE_URL = 'https://32.Javascript.htmlacademy.pro/kekstagram'; | |||
| const Route = { | |||
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.
С заглавной буквы именуем класс или компонент. Подобные константы, обычно именуются в UPPER_CASE стиле
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.
в курсе пишется, что обьект с константами пишется так же с большой буквы
js/main.js
Outdated
| @@ -1,5 +1,4 @@ | |||
| import { renderThumbnails } from './thumbnail_render.js'; | |||
| import './thumbnail_render.js'; | |||
| import './photo_upload_form.js'; | |||
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.
Импортировать необходимо функцию/функции и их запускать. Так действие будет более предсказуемым. Это важно для всех импортов.
Без имени импортируем только сторонние библиотеки, если это предусмотрено их документацией. Например, бутстрап
js/photo_upload_form.js
Outdated
| import { showAlertSucces } from './alert-success.js'; | ||
| import {showAlertError} from './alert-error.js'; | ||
|
|
||
| const GET_ERROR_TAGS = 'tags'; |
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.
Это именование полезно обсудить. У констант не ожидается глагол.
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.
исправил
js/popup-error.js
Outdated
| setTimeout(removePopup, 5000); | ||
| }; | ||
|
|
||
| function removePopup(){ |
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.
Стрелочная функция возможна в этом месте?
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.
поправил
js/thumbnail_render.js
Outdated
| @@ -1,8 +1,9 @@ | |||
| import { createDescriptionPhoto } from './data.js'; | |||
| import {openPost} from './post.js'; | |||
| import {showPopup} from './popup-error.js'; | |||
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.
надо проверить линтер и его правила. В части модулей есть пробелы между скобок, здесь нет
js/thumbnail_render.js
Outdated
| picturesContainer.append(fragment); | ||
| }; | ||
|
|
||
| getData() |
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.
Действие выполнится при импорте функций из модуля. В последовательности кода не будет места загрузки данных и найти это место будет сложно.
Название модуля "рендер" - значит он не про загрузку, а про отображение
Где-то ранее, например в модуле инициализации, выполняется загрузка данных и при успехе вызывается рендер.
🎓 Надо подкачаться