Code review 2 #2
Conversation
| const initHTML = () => { | ||
|
|
||
| const body = document.body; | ||
| body.innerHTML += |
There was a problem hiding this comment.
Для этих целей нужно использовать Element: insertAdjacentHTML()
Работа с innerHTML приводит к большим проблемам, например см. первый ответ здесь: https://stackoverflow.com/questions/2684956/addeventlistener-gone-after-appending-innerhtml
|
|
||
| const body = document.body; | ||
| body.innerHTML += | ||
| ' <div class="page fixed disable refactor-page">\n' + |
There was a problem hiding this comment.
Все эти имена классов: page fixed disable refactor-page слишком общие, чтобы использовать в модульном решении, которое сейчас разрабатывается. Есть большая вероятность, что на встраиваемой странице уже будут использоваться такие имена классов, что приведет к переопределению CSS-свойств и другим конфликтам.
There was a problem hiding this comment.
Как вариант, добавить всем используемым классом единый уникальный префикс.
| body.innerHTML += | ||
| ' <div class="page fixed disable refactor-page">\n' + | ||
| ' <div class="refactor-background"></div>\n' + | ||
| ' <div class="refactor-container" onselectstart="return false">\n' + |
There was a problem hiding this comment.
Более правильно добавлять обработчики события через addEventListener: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
| } | ||
| initHTML(); | ||
|
|
||
| const form = document.querySelector('form.editAvatar'); |
There was a problem hiding this comment.
Неудачное решение, что здесь происходит работа с формой: это делает модуль не универсальным. Грубо говоря, он будет работать только с одной конкретной формой.
Более универсальный подход - это ограничиться элементом <input>, в котором происходит работа с файлом и в момент, когда файл ужимается до необходимого размера, подменять значение files у этого элемента. Пример кода можно подсмотреть здесь: https://stackoverflow.com/questions/1696877/how-to-set-a-value-to-a-file-input-in-html-to-a-client-side-disk-file-system-pat/70485949#70485949
Что-то типа этого:
let fileName = 'hasFilename.jpg'
let file = new File([imgBlob], fileName,{type:"image/jpeg", lastModified:new Date().getTime()}, 'utf-8');
let container = new DataTransfer();
container.items.add(file);
document.querySelector('#file_input').files = container.files;
| let targetWidth; | ||
| let targetHeight; |
There was a problem hiding this comment.
Почему эти переменные определяются ниже?
Почему не можем сразу сделать:
const fileInput = document.querySelector(".imageResizeAndCrop");
const targetWidth = Number(fileInput.getAttribute('data-target-width'));
const targetHeight = Number(fileInput.getAttribute('data-target-height'));
| .addEventListener("click", () => { | ||
| zoomVariant = type; | ||
| setEnableZoomVariant(); | ||
| }, type); |
There was a problem hiding this comment.
Здесь точно нужен третий параметр в addEventListener?
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
| setDifferentScaleTypesMethod(); | ||
|
|
||
| const saveUpdatedImageEvent = () => { | ||
| const crop_button = document.querySelector(".refactor-container__button_type-confirm"); |
There was a problem hiding this comment.
Почему внезапно snake_case в имени переменной?
| console.log(blob); | ||
| dt.items.clear(); | ||
| dt.items.add(new File([blob], 'ava.png', {type: "image/png"})); | ||
| form.requestSubmit(); |
There was a problem hiding this comment.
Выше уже написал, почему не нужно смешивать логику обрезки картинки и логику отправки формы.
| const denyUpdatedImageEvent = () => { | ||
| window.removeEventListener("resize", updateImageAfterWindowResize); | ||
|
|
||
| const removeUpdates = () => { | ||
| imageRefactorPage.classList.add("disable"); | ||
| } | ||
|
|
||
| document.querySelector(".refactor-container__button_type-deny").addEventListener("click", removeUpdates); | ||
| document.querySelector(".refactor-container__close-button").addEventListener("click", removeUpdates); | ||
| } | ||
| denyUpdatedImageEvent(); |
There was a problem hiding this comment.
Я правильно понял, что здесь вы определили функцию просто чтобы ее один раз вызвать?
| form.addEventListener('submit', async (evt) => { | ||
| evt.preventDefault(); | ||
| const username = document.querySelector('section.auth').dataset.username; | ||
| form.querySelector('input[name="username"]').setAttribute('value', username); | ||
| const file_list = dt.files; | ||
| console.log('Коллекция файлов создана:'); | ||
| console.dir(file_list); | ||
| form.querySelector('input[name="avatar"]').files = file_list; | ||
| const response = await window.acomicsLegacyClient.sendFormAndParseHtml(evt.target); | ||
| console.log(response); | ||
| }); |
There was a problem hiding this comment.
Выше написал, почему не стоит смешивать логику обрезки картинки с логикой отправки формы.
2 participants
No description provided.