Правда или действие #10

NikitaKir98 · 2025-05-21T13:51:01Z

ILokalin · 2025-05-22T14:26:41Z

js/photo_upload_form.js

+  }
+}
+
+function onDescriptionInput () {


ожидаются стрелочные функции - надо проверить весь код

убрал фанкшен декларейшен

ILokalin · 2025-05-22T14:28:43Z

js/validation_description.js

+const validateDescription = (value) => {
+  errorMessage = '';
+
+  const inputText = value;


с большей вероятностью, нет необходимости в новой константе - используем имеющуюся

убрал константу

ILokalin · 2025-05-22T14:30:24Z

js/validation_description.js

@@ -0,0 +1,33 @@
+const DESCRIPTION_SYMBOLS_MAX = 140;
+
+let errorMessage = '';


Скорее всего, в этой переменной нет необходимости - экспортировать ее нельзя, а функции валидации обычно детерминированные

убрал переменную, сделал обьект

ILokalin · 2025-05-22T14:36:32Z

js/validation_description.js

+    }
+  ];
+
+  return rules.every((rule) => {


Можно вернуть код ошибки, если она есть или false (null), если ошибки нет

функция переделана

ILokalin · 2025-05-22T14:37:29Z

js/validation_description.js

+  });
+};
+
+const errorDescription = () => errorMessage;


ошибку удобнее получить от функции валидации

исправлено на const getError = (field) => () => errors[field]

ILokalin · 2025-05-22T14:38:45Z

js/validation_description.js

+const errorDescription = () => errorMessage;
+
+
+export { validateDescription, errorDescription };


функции валидации удобнее размесить в одном модуле validate.js

обьединил модули

ILokalin · 2025-05-22T14:40:11Z

js/validation_description.js

+    return true;
+  }
+
+  const rules = [


Вот это можно обсудить, чтобы реализовать более оптимальное решение

обсудили, исправили

ILokalin · 2025-05-22T14:42:38Z

js/photo_upload_form.js

+function onHashtagsInput () {
+  if (pristine.validate()) {
+    imgUploadSubmit.disabled = false;
+  } else {


секция else избыточная

if (условие) { // действие return } // действие

исправил

ILokalin · 2025-05-22T14:44:49Z

js/photo_upload_form.js

+}
+
+const onDocumentKeydown = (event) => {
+  if (isEscapeKey(event)) {


сначала отсекаем ненужные кейсы

if (!isEscapeKey) { return; }

После упрощаем условие, отсекая else

поправил немного по другому, нужно проверить

через тернарный оператор

ILokalin · 2025-05-22T14:46:16Z

js/photo_upload_form.js

+const onDocumentKeydown = (event) => {
+  if (isEscapeKey(event)) {
+    if (document.activeElement === inputHashtags || document.activeElement === inputDescription) {
+      event.stopPrepagation();


Точно ли необходимо останавливать распространение? Если не убрать, на что может повлиять?

сделал без stopPrepagation();

ILokalin · 2025-05-29T18:55:14Z

js/photo_upload_form.js

+const body = document.querySelector('body');
+const overlayForm = document.querySelector('.img-upload__overlay');
+const closeBtnForm = overlayForm.querySelector('.img-upload__cancel');
+const UploadForm = document.querySelector('.img-upload__form');


Именование констант и переменных начинается со строчных букв - camelCase.
С прописных именуются компоненты и классы - PascalCase
Неизменные константы пишем прописными - UPPER_CASE

исправил

ILokalin · 2025-05-29T18:55:24Z

js/photo_upload_form.js

+const UploadForm = document.querySelector('.img-upload__form');
+const inputDescription = UploadForm.querySelector('.text__description');
+const inputHashtags = UploadForm.querySelector('.text__hashtags');
+const UploadSubmit = UploadForm.querySelector('.img-upload__submit');


Именование констант и переменных начинается со строчных букв - camelCase.
С прописных именуются компоненты и классы - PascalCase
Неизменные константы пишем прописными - UPPER_CASE

исправил

ILokalin · 2025-05-29T18:55:59Z

js/photo_upload_form.js

+  if (pristine.validate()) {
+    UploadSubmit.disabled = false;
+    return;
+  } UploadSubmit.disabled = true;


Новая команда после условия - полезно разместить с новой строки

ILokalin · 2025-05-29T18:57:45Z

js/photo_upload_form.js

+  } UploadSubmit.disabled = true;
+};
+
+const onDocumentKeydown = (event) => isEscapeKey(event) && (document.activeElement === inputHashtags || document.activeElement === inputDescription ? event.stopPropagation() : closeFormUpload());// eslint-disable-line


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

В данной ситуации удобнее использовать условие if:

от этой функции не ожидается возвращаемый результат - т.е., return в ней не требуется

вызываемые в тернарном операторе функции тоже не возвращают какой-либо результат

В таких случаях:

if (условие) { действиеА return; } действиеБ

Есть ли необходимость останавливать распространение события?

исправил с if, но нужно обсудить

функция переписана без остановки события и без тернарного оператора

ILokalin · 2025-05-29T18:58:24Z

js/photo_upload_form.js

+
+const onDocumentKeydown = (event) => isEscapeKey(event) && (document.activeElement === inputHashtags || document.activeElement === inputDescription ? event.stopPropagation() : closeFormUpload());// eslint-disable-line
+
+const onClickCloseBtnForm = () => closeFormUpload();// eslint-disable-line


Так же - полезно указать, какое правило отменяется. Сейчас неочевидна причина отмены

Можем использовать closeFormeUpload без этой функции?

тогда в функции closeFormeUpload, она будет использовать сама себя

теперь причина отмены понятна, функция сделана через if

closeFormUpload - мы не можем использовать без этой функции
при удалении обработчика мы используем onClickCloseBtnForm внутри closeFormUpload

ILokalin · 2025-05-29T18:58:59Z

js/photo_upload_form.js

+const onDocumentKeydown = (event) => isEscapeKey(event) && (document.activeElement === inputHashtags || document.activeElement === inputDescription ? event.stopPropagation() : closeFormUpload());// eslint-disable-line
+
+const onClickCloseBtnForm = () => closeFormUpload();// eslint-disable-line
+


Принято не оставлять в коде более одной пустой строки

оставил две строки только в тех местах, где логическое отделение блоков кода

исправлено

ILokalin · 2025-05-29T19:09:10Z

js/photo_upload_form.js

+
+uploadFile.addEventListener('change', onClickFormUpload);
+
+pristine.addValidator(inputHashtags, validateHashtags, getError('tags'), 1, false);


Имена полей (tags и description) и значения 1, 2 - это было бы полезно вынести в константы. Предлагаю это обсудить отдельно

приоритет валидации (1 и 2) убрал, не нашел смысловой нагрузки в нем.

значения 1 и 2 убрал, они отвечали за приоритет валидации, однако в ТЗ не указан приоритет валидации

добавил константы, нужно обсудить

ILokalin · 2025-05-29T19:11:29Z

js/validation.js

+};
+
+const validateHashtags = (value) => {
+


много пустых строк - разряженный текст сложно читать:

пустые строки отделяют функции - перед и после

пустую строку часто оставляем перед return

можем отделить крупные логические блоки

поправил

убрал избыточные пустые строки

ILokalin · 2025-05-29T19:12:11Z

js/validation.js

+
+
+const validateDescription = (inputText) => {
+


Можем исключить пустую строку перед первым условием в функции

убрал пустую строку

ILokalin · 2025-05-29T19:14:29Z

js/validation.js

+    }
+  ];
+
+  return rules.every((rule) => {


Избыточная конструкция. В данном случае она повторяет код другой функции и напрашивается вынесение этого решения в отдельную функцию.

Чтобы такого желания не было:

if (inputText.length > DESCRIPTION_SYMBOLS_MAX) { действие }

исправил

ILokalin · 2025-05-29T19:15:01Z

js/validation.js

+    return true;
+  }
+
+  const rules = [


Здесь список не требуется

исправил

ILokalin · 2025-05-29T19:20:43Z

js/validation.js

+    return true;
+  }
+
+  const inputArray = inputText.split(/\s+/);


Неполезно использовать венгерскую нотацию - не добавляем в названия сущностей их тип. Когда задаем вопрос "что это?", ключевым должно оказать название сущности, не тип

изменил нейминг на inputData

ILokalin · 2025-05-29T19:24:26Z

js/validation.js

+
+  return rules.every((rule) => {
+    const isInvalid = rule.check;
+    if (isInvalid) {


Можем упростить и исключить константу

if (rule.check) { ... return false; } return true;

упростил и исключил константу

ILokalin · 2025-05-29T19:38:41Z

js/validation.js

+      error: 'Хэштеги разделяются пробелами.'
+    },
+    {
+      check: inputArray.some((item, num, arr) => arr.includes(item, num + 1)),


num полезно заменить на i - буквой i именуем индекс

arr - можем не использовать аргумент, так как в замыкании есть исходная коллекция

поправил замечание

Выполнил ДЗ по модулю 9.1

f554971

keksobot changed the title ~~Выполнил ДЗ по модулю 9.1~~ Правда или действие May 21, 2025

ILokalin reviewed May 22, 2025

View reviewed changes

NikitaKir98 added 2 commits May 23, 2025 15:18

исправление замечаний

3a6c072

исправил замечания

ad4cb35

ILokalin reviewed May 29, 2025

View reviewed changes

NikitaKir98 added 2 commits June 2, 2025 13:32

поправил замечания

4891e8d

поправил исключение правил для линтера

25bb83b

ILokalin merged commit d1002f0 into htmlacademy-javascript:master Jun 2, 2025
1 check passed

		@@ -0,0 +1,33 @@
		const DESCRIPTION_SYMBOLS_MAX = 140;

		let errorMessage = '';

		const errorDescription = () => errorMessage;


		export { validateDescription, errorDescription };


		const onDocumentKeydown = (event) => isEscapeKey(event) && (document.activeElement === inputHashtags \|\| document.activeElement === inputDescription ? event.stopPropagation() : closeFormUpload());// eslint-disable-line

		const onClickCloseBtnForm = () => closeFormUpload();// eslint-disable-line


		uploadFile.addEventListener('change', onClickFormUpload);

		pristine.addValidator(inputHashtags, validateHashtags, getError('tags'), 1, false);

Правда или действие #10

Правда или действие #10

Uh oh!

Conversation

NikitaKir98 commented May 21, 2025 • edited by keksobot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

NikitaKir98 commented May 21, 2025 •

edited by keksobot

Loading