-
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
Открывается и закрывается #8
Conversation
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.
Из названия не понятно что содержится в comments.js. По идее там должно лежать представление комментариев, а не их данные. Если решил разделить данные то назови comments-data.js или data-comments.js
bigPhotoViewer.js -> big-photo-viewer.js
index.html
Outdated
@@ -6,7 +6,7 @@ | |||
<link rel="stylesheet" href="css/normalize.css"> | |||
<link rel="stylesheet" href="css/style.css"> | |||
<link rel="shortcut icon" href="favicon.ico" type="image/x-icon"> | |||
<title>Кекстаграм</title> | |||
<title>Кексограм</title> |
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/bigPhotoViewer.js
Outdated
@@ -0,0 +1,57 @@ | |||
import {photoData} from './data.js'; | |||
|
|||
const picturesContainer = document.querySelector('.pictures'); |
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.
Здесь и далее нарушен критерий Д6
js/bigPhotoViewer.js
Outdated
bigPhoto.querySelector('.social__comment-count').classList.add('hidden'); | ||
bigPhoto.querySelector('.comments-loader').classList.add('hidden'); | ||
|
||
bigPhoto.querySelector('img').src = targetPhoto.url; |
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 isPressed = element.getAttribute('aria-pressed', false);
-> bigPhoto.querySelector('img').setAttribute('src', targetPhoto.url);
js/bigPhotoViewer.js
Outdated
|
||
picturesContainer.addEventListener('click', (evt) => { | ||
const targetPhotoId = evt.target.closest('.picture'); | ||
const targetPhoto = photoData.find((photo) => photo.photoId === Number(targetPhotoId.dataset.photoId)); |
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.
targetPhoto нужно деструктуризировать
js/bigPhotoViewer.js
Outdated
|
||
targetPhoto.comments.forEach((comment) => { | ||
const newComment = commentTemplate.cloneNode(true); | ||
newComment.querySelector('.social__picture').src = comment.avatar; |
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.
newComment.querySelector('.social__picture') ищется дважды. Критерий Д21
js/data.js
Outdated
@@ -28,7 +28,7 @@ const PHOTO_DESCRIPTIONS = ['Закат над океаном', | |||
'Маленькая хижина в горах', 'Закат в тропиках', 'Стадо оленей на рассвете', 'Городской парк с фонтаном', | |||
'Облачный горизонт над мегаполисом', 'Гребной канал с лодками', 'Старинный замок в горах']; | |||
|
|||
function getRandomPhotoObjects(objectsCount) { | |||
const photoData = (function getPhotoData(objectsCount) { |
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
import {showThumbnail} from './thumbnails.js'; | ||
import './bigPhotoViewer.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/bigPhotoViewer.js
Outdated
@@ -0,0 +1,57 @@ | |||
import {photoData} from './data.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.
Мы разделяем данные от представления. Данные и представление объявляются в main.js. Данные в main.js должны передаваться как атрибут в big-photo
Файлы в проекте должны называться кебабкейсом, а не кэмэлкейсом
js/bigPhotoViewer.js
Outdated
const commentTemplate = socialComments.querySelector('.social__comment'); | ||
const body = document.body; | ||
|
||
socialComments.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.
Комментарии должны перетираться в файле с комментариями, а не в файле с детальной фотографии
js/data.js
Outdated
photoData = getPhotoData(count); | ||
} | ||
|
||
export {photoData, generateMockData}; |
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.
А почему нельзя просто экспортировать getPhotoData
который возвращает массив объектов?
С 83 строки по 87 абсолютно лишний код
js/main.js
Outdated
|
||
const OBJECTS_COUNT = 26; | ||
generateMockData(26); |
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.
import { getPhotoData } from './data.js';
const photoData = getPhotoData(26);
js/main.js
Outdated
@@ -1,8 +1,8 @@ | |||
import {getRandomPhotoObjects} from './photos.js'; | |||
import {photoData, generateMockData} from './data.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/thumbnails.js
Outdated
pictureElement.querySelector('img').src = url; | ||
pictureElement.querySelector('img').alt = description; | ||
pictureElement.dataset.photoId = photoId; | ||
pictureElement.querySelector('img').setAttribute('src', url); |
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.
Дважды дублируется pictureElement.querySelector('img')
Плохо искать по тегу <img>
, там может быть больше одной картинки и будет выбрана ближайшая
js/data.js
Outdated
function getRandomPhotoObjects(objectsCount) { | ||
let photoData = []; | ||
|
||
// const photoData = (function getPhotoData(objectsCount) { |
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/thumbnails.js
Outdated
const pictureElement = pictureTemplateElement.cloneNode(true); | ||
|
||
pictureElement.querySelector('img').src = url; | ||
pictureElement.querySelector('img').alt = description; | ||
pictureElement.dataset.photoId = photoId; |
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.
setAttribute забыл для pictureElement.dataset.photoId = photoId;
|
||
function fillCommentsList(comments) { | ||
socialCommentsElement.innerHTML = ''; | ||
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.
Деструктуризируй comment
Добавляет решение задания "Открывается и закрывается Ч.1"
🎓 Открывается и закрывается