Skip to content

Week 3#6

Open
vikashatalova wants to merge 11 commits intoCodegirlSchool:mainfrom
vikashatalova:week-3
Open

Week 3#6
vikashatalova wants to merge 11 commits intoCodegirlSchool:mainfrom
vikashatalova:week-3

Conversation

@vikashatalova
Copy link
Copy Markdown

No description provided.

scripts/main.js Outdated

if (pattern.test(postTextValue) && postTextValue.length <= count) {
textValidate.textContent = 'Данные введены корректно';
const sumsCounter = Number(2000) - Number(postTextValue.length);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем ты пишешь Number(2000), если 2000 уже изначально число?

scripts/main.js Outdated
if (pattern.test(hashtags) && hashtags.length <= 200) {
textValidateTags.textContent = 'Данные введены корректно';
postPublishButton.disabled = false;
} else if (hashtags.trim() === ' ') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можешь пояснить, как хэштеги после trim() будут равны пустой строке, если trim удаляет незначащие пробелы с двух сторон?

scripts/main.js Outdated
sum = 'Вы вели слишком много символов введите на '+sums+' меньше';
postText.style.outlineColor = 'var(--error)'
textValidateTags.textContent = 'Данные введены неверно';
postHashtags.style.outlineColor = 'var(--error)';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему ты не хочешь завести CSS-класс и добавлять его на postHashtags в случае не валидного значения?

scripts/main.js Outdated

comments.forEach((comment) => {
commentsContent.innerHTML = `
<div class="comments__item">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше использовать шаблон (template), а не вставлять куски HTML. При такой записи легко опечататься, и тогда результат будет непредсказуемым

scripts/main.js Outdated
<div class="comments__item">
<img
class="comments__item-avatar"
src="https://avatars.dicebear.com/api/avataaars/${(Math.random() + 1).toString(36).substring(7)}.svg"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Адреса нужно выносить в константу

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scripts/main.js Outdated
commentsButton.addEventListener('click', () => {
const comment = postComment.value;
if (comment.trim() === '') {
console.log('error');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вывода в консоль быть не должно

scripts/main.js Outdated
const likeButton = document.querySelector('.fa-heart');
const countElement = document.querySelector('#like');
const statisticsLikes = document.querySelector('.statistics__likes');
let likes = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У тебя одна глобальная переменная с количеством лайков на все посты?

scripts/main.js Outdated
let likes = 0;

likeButton.addEventListener('click', () => {
likes++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тебе нужно сначала отправить запрос на сервер, и только когда он ответит, что все ок, ты увеличиваешь количество лайков

scripts/main.js Outdated
getItemId();
});

const countLikesPost = (content) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Точно ли нужна под это функция?

scripts/main.js Outdated
};

const updateCount = (itemId) => {
countElement.innerHTML = likes;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Используешь глобальную переменную для лайков. Не надо так

scripts/main.js Outdated
const imageAvatar = document.querySelector('#profile-avatar');

const getBioUser = () => {
const response = fetch('https://c-gallery.polinashneider.space/api/v1/users/me/', {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не вынесла URL в константу

scripts/main.js Outdated
const formData = new FormData();
formData.append("photo", filUploadAvatar.files[0]);

fetch('https://c-gallery.polinashneider.space/api/v1/users/me/', {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

И тут

scripts/main.js Outdated
textValidate.textContent = 'Поле обязательно для заполнения';
postPublishButton.disabled = true;
} else if (postTextValue.length === 2000) {
const sumsCounter = Number(2000) - Number(postTextValue.length);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Штуковину с Number(2000) нужно поправить везде

scripts/main.js Outdated
} else if (postTextValue === '') {
textValidate.textContent = 'Поле обязательно для заполнения';
postPublishButton.disabled = true;
} else if (postTextValue.length === 2000) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У тебя же есть переменная count. Почему бы в коде не использовать ее?

postPublishButton.disabled = true;
} else {
textValidateTags.textContent = 'Данные введены неверно';
postHashtags.classList.add('error-validate');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

scripts/main.js Outdated

const inputsAddPublish = document.querySelectorAll('textarea');

for (let i = 0; i < inputsAddPublish.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можешь пояснить, зачем тебе тут цикл? Получается, ты inputsAddPublish.length раз сбрасываешь значения одних и тех же элементов. У тебя даже i в теле цикла не фигурирует

scripts/main.js Outdated
document.body.classList.remove('with-overlay');
getPostUsers();

showMessage('#alert-success');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У тебя всегда один и тот же текст показывается в случае успеха?

scripts/main.js Outdated
if (isHidden) {
loader.removeAttribute('hidden');
} else {
loader.setAttribute('hidden', '');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У вас есть критерий про использование одного API. Где-то ты используешь el.disabled = true, а где-то el.setAttribute('hidden', '')

Нужно унифицировать. То есть либо через точку обращаться к атрибуту, либо через setAttribute

});
}
createAvatarUser();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Много пустых строк в конце файла. Зачем?

scripts/utils.js Outdated
commentsButton.addEventListener('click', () => {
const comment = postComment.value;
if (comment === '') {
showMessage('#alert-fail');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если комментарий пустой, можно не показывать уведомление, а просто блокировать кнопку добавления, например

scripts/utils.js Outdated
commentsContent.dataset.postId = id;

comments.forEach((comment) => {
const dateCommnets = new Date(comment.created_at);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Опечатка

scripts/utils.js Outdated

const commentsAvatar = document.createElement('img');
commentsAvatar.className = 'comments__item-avatar';
commentsAvatar.src = `https://avatars.dicebear.com/api/avataaars/${(Math.random() + 1)}.svg`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Такое лучше выносить в константу (адрес) — я вроде про это писала

scripts/utils.js Outdated
commentTime.className = 'comments__item-time';
commentTime.innerText = newDateComments;

commentsText.append(commentNickname);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Возможно, тебе было бы удобнее использовать template, а не собирать такую сложную матрешку. Вы можете менять верстку на свое усмотрение

scripts/utils.js Outdated
showMessage('#alert-fail');
})
.finally(() => {
previewPostModal.classList.remove('active');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Выглядит как повторяющийся код, который можно вынести в функцию

if (comment === '') {
showMessage('#alert-fail');
} else {
sendComment(comment, id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У меня почему-то не срабатывает добавление комментариев, но, может, на пейджес старая версия

Copy link
Copy Markdown

@PolinaShneider PolinaShneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ты все еще пишешь код в реквесте, который открыт в репозиторий школы. Почти все хорошо, но тебе нужно открыть его в свой репозиторий, иначе защита проекта будет невозможна 😿

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