-
Notifications
You must be signed in to change notification settings - Fork 1
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
Открывается и закрывается #7
Открывается и закрывается #7
Conversation
js/main.js
Outdated
|
||
appendPhotos(createPhotos(25)); | ||
const result = createPhotos(25); |
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.
У нас функция называется createPhotos, и возвращает она photos, а не result, как вариант можно photosData или просто data
js/modal.js
Outdated
const likesCountElement = bigPictureElement.querySelector('.likes-count'); | ||
const commentTotalElement = bigPictureElement.querySelector('.social__comment-total-count'); | ||
const socialCommentsElement = bigPictureElement.querySelector('.social__comments'); | ||
|
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.
Внутри квадратных скобок пишется индекс массива, а не id
js/modal.js
Outdated
const cancelElement = bodyElement.querySelector('.big-picture__cancel'); | ||
|
||
const renderElements = (photos, id) => { | ||
|
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 { url, description, likes, comments } = photos[id];
js/modal.js
Outdated
likesCountElement.textContent = photos[id].likes; | ||
commentTotalElement.textContent = photos[id].comments.length; | ||
|
||
photos[id].comments.forEach((comment) => { |
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.
Отрисовку комментариев нужно вынести в другой файл, этот файл и так слишком перегружен. Сейчас нарушается критерий Д25
js/modal.js
Outdated
commentTotalElement.textContent = photos[id].comments.length; | ||
|
||
photos[id].comments.forEach((comment) => { | ||
const listItem = document.createElement('li'); |
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.
Любой item является элементом списка (list), потому вместо listItem можно писать короче Item. Но все равно не понятно чей это элемент, можно commentItem. Но по критериям нам желательно для DOM-элементов задавать суффикст Element. Я б вообще его назвал бы просто, commentElement
js/modal.js
Outdated
bigPictureElement.removeEventListener('click', onBigPictureElementClick); | ||
}; | ||
|
||
function onCancelElementClick() { |
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.
Дополнительные функции не нужны. На кнопку Отмены можно повесить closeModal
js/main.js
Outdated
const result = createPhotos(25); | ||
|
||
appendPhotos(result); | ||
loadModal(result); |
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.
loadModal переводится как загрузить модальное окно. Назови лучше renderBigPicture или renderPhotoDetail
js/main.js
Outdated
appendPhotos(createPhotos(25)); | ||
const result = createPhotos(25); | ||
|
||
appendPhotos(result); |
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.
renderPhotos или renderPhotosList
js/modal.js
Outdated
photos[id].comments.forEach((comment) => { | ||
const listItem = document.createElement('li'); | ||
listItem.classList.add('social__comment'); | ||
listItem.innerHTML = |
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.
Не надо это делать через innerHTML, код должен быть единообразным, как в preview.js
🎓 Открывается и закрывается