-
Notifications
You must be signed in to change notification settings - Fork 1
Перламутровые пуговицы #14
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
Перламутровые пуговицы #14
Conversation
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
1 similar comment
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
js/upload-picture.js
Outdated
|
|
||
| function onDocumentKeydown (evt) { | ||
| if (isEscapeKey(evt.key) && !isInputFocused()) { | ||
| const isErrorMessageOpen = document.querySelector('.error'); |
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.
Можно избавиться от этой части, описано будет в dialogs
js/filter.js
Outdated
| return shuffled.slice(0, MAX_RANDOM_PHOTO); | ||
| }; | ||
|
|
||
| export const resetActiveButtons = (arr) => { |
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.
Ты можешь использовать прием, который у тебя с effects, и запоминать текущий фильтр, если же тебе он не нравится, то не нужно делать forEach, ты можешь найти по классу нужный фильтр и снять с него active
js/filter.js
Outdated
|
|
||
| const MAX_RANDOM_PHOTO = 10; | ||
|
|
||
| export const shuffleArray = (array) => { |
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.
Как вариант, но есть проще, через toSorte, он не идеальный, но для наших задач подойдет.
| @@ -0,0 +1,89 @@ | |||
| import { isEscapeKey } from './utils.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.
Этот модуль нужно переделать:
1 - функция открытия диалогов и закрытия у тебя очень похожи, не считая errorMessageGetPhotos, поэтому вынеси общую логику, постарайся оставить 2 функции openDialog / closeDialog, ты можешь указывать параметром template который открыть, явно передавая его, за них отвечает messageElement
2 - все поиски элементов сделай за переделами функций
const messageElement = document.querySelector('.error');
и подобное, нам не нужно каждый раз при открытии искать элемент, ты их знаешь заранее, достаточно клонировать
3 - функции - это глагол, сейчас у тебя это не так: errorMessageSendPhoto, errorMessageGetPhotos и тд
4 - ты можешь запоминать последний открытый dialog в переменную, и потом этим воспользоваться в onOutsideClick, тебе не нужно искать постоянно элемент, ты его знаешь, а по закрытию очищаешь
dialog.remove();
dialog = null
5 - чтобы не делать проверку isErrorMessageOpen, ты можешь в onDocumentKeydown добавить вызов дополнительной функции, которая отвечает за отмену всплытия события
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
js/dialogs.js
Outdated
| const errorElement = dataErrorTemplate.content.cloneNode(true); | ||
| document.body.appendChild(errorElement); | ||
|
|
||
| const errorMessage = document.querySelector('.data-error'); |
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.
errorMessage === errorElement, не нужно искать то, что ты уже нашел
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
|
♻️ Я собрал ваш пулреквест. Посмотреть можно здесь. |
🎓 Перламутровые пуговицы
💥 https://htmlacademy-javascript.github.io/2252139-kekstagram-2/14/