Aleksey Titekin - project completed #97
Conversation
| import SortableList from '../sortable-list/index.js'; | ||
| import fetchJson from "../../utils/fetch-json.js"; | ||
|
|
||
| const BACKEND_URL = `${process.env.BACKEND_URL}` |
There was a problem hiding this comment.
👍 good
лайк за использование переменных окружения
| } | ||
| } | ||
|
|
||
| constructor() { |
There was a problem hiding this comment.
не оставляйте пустой constructor - удалите его
|
|
||
| getSubCategories(subCategories) { | ||
| const items = subCategories.map( elem => this.getElementList(elem.id, elem.title, elem.count)); | ||
| const elementList = new SortableList({items: items}); |
There was a problem hiding this comment.
тут можно сократить:
const elementList = new SortableList({items});
| url.pathname = '/api/rest/categories'; | ||
| url.searchParams.set('_sort','weight'); | ||
| url.searchParams.set('_refs','subcategory'); | ||
| return await this.loadData(url); ; |
| } | ||
|
|
||
| async getCategories() { | ||
| const url = new URL(BACKEND_URL); |
There was a problem hiding this comment.
const url = new URL('/api/rest/categories', BACKEND_URL);
| return await this.loadData(url); ; | ||
| } | ||
|
|
||
| async loadData(url) { |
There was a problem hiding this comment.
async loadData(url) {
return await fetchJson(url);
}
| } | ||
|
|
||
| initEventListeners() { | ||
| Object.values(this.subElements).forEach( value => { |
There was a problem hiding this comment.
может воспользоваться подходом event-delegation?
чтобы не навешивать обработчики в цикле, а обойтись только одним
| @@ -0,0 +1,123 @@ | |||
| export default class DoubleSlider { | |||
| @@ -0,0 +1,49 @@ | |||
| export default class NotificationMessage { | |||
| static activeElement = null; | |||
| } | ||
|
|
||
| render() { | ||
| let element = document.createElement('div'); |
|
|
||
| show(root = document.body) { | ||
| if (NotificationMessage.activeElement) { | ||
| this.destroy.call(NotificationMessage.activeElement); |
There was a problem hiding this comment.
🔴 please don't do this
нужно сделать вот так:
NotificationMessage.activeElement.destroy();
| link = '', | ||
| value = 0 | ||
| url = "", | ||
| range = { from: null, to: null }, |
There was a problem hiding this comment.
range = {
from: new Date(),
to: new Date()
}
|
|
||
| getSubElements (element) { | ||
| const elements = element.querySelectorAll('[data-element]'); | ||
| const data = await fetchJson(url); |
|
|
||
| imageListContainer.firstElementChild.append(this.getImageItem(result.data.link, file.name)); | ||
| onUploadChange = async event => { |
| const productPromise = this.productId | ||
| ? this.loadProductData(this.productId) | ||
| : Promise.resolve([this.defaultFormData]); | ||
| const [[ product ], categories ] = await Promise.all([promiseProduct, promiseCategories]); |
| this.subElements = this.getSubElements(element); | ||
| categories.forEach( category => { | ||
| category.subcategories.forEach(subCategory => { | ||
| const option = new Option(`${category.title} > ${subCategory.title}`, subCategory.id); |
| async loadProductData(productId) { | ||
| return await fetchJson(`${process.env.BACKEND_URL}api/rest/products?id=${productId}`); | ||
| getProduct() { | ||
| const url = new URL(BACKEND_URL); |
There was a problem hiding this comment.
не делайте так, не присваивайте значение в pathname
сразу конструируйте url:
const url = new URL('/api/rest/products', BACKEND_URL);
| const url = new URL(BACKEND_URL); | ||
| url.pathname = '/api/rest/products'; | ||
| url.searchParams.set('id', this.productId); | ||
| const product = (this.productId) ? this.loadData(url) : [Promise.resolve(this.defaultFormData)]; |
| async loadCategoriesList() { | ||
| return await fetchJson(`${process.env.BACKEND_URL}api/rest/categories?_sort=weight&_refs=subcategory`); | ||
| getCategories() { | ||
| const url = new URL(BACKEND_URL); |
There was a problem hiding this comment.
url лучше сконструировать в constructor чтобы каждый раз при вызове getCategories не пересоздавать данное значение
|
|
||
| try { | ||
| const result = await fetchJson(url, { | ||
| method: this.productId ? 'PATCH' : 'PUT', |
|
|
||
| const items = images.map(({url, source}) => this.getImageItem(url, source)); | ||
| dispatchEvent (id) { | ||
| const event = new CustomEvent( |
|
|
||
| constructor({items = []} = {}) { | ||
| this.items = items; | ||
| elem = event.target.closest('[data-grab-handle]'); |
There was a problem hiding this comment.
🔴 please don't do this
не делайте так! Вы используете одну и ту же переменную для разных значений
|
|
||
| this.placeholderElem.style.width = itemElem.style.width; | ||
| this.placeholderElem.style.height = itemElem.style.height; | ||
| this.current.style.left = this.xPosition + 1 + 'px'; |
There was a problem hiding this comment.
1 следует вынести в отдельную переменную
|
|
||
| if (rect.bottom < document.documentElement.clientHeight && !this.loading && !this.isSortLocally) { | ||
|
|
||
| this.start = this.end + 1; |
There was a problem hiding this comment.
что такое 1?
может вынести это в отдельную переменную и дать соответствующее имя?
| } | ||
| sortData() { | ||
| return this.data.sort((a, b) => { | ||
| [a, b] = [a[this.fieldValue], b[this.fieldValue]]; |
There was a problem hiding this comment.
🔴 please don't do this
пожалуйста не мутируйте параметры ф-ций, это плохая практика, да и читается
такой код хуже, чем выделение дополнительных переменных
| </div>` | ||
| ).join(''); | ||
| } | ||
| switch(sortType) { |
| if (!elem) return; | ||
| this.message = elem.dataset.tooltip; | ||
| elem.addEventListener("pointermove", this.onMove); | ||
| elem.addEventListener("pointerout", this.onOut, { once: true }); |
| document.removeEventListener('pointerover', this.onMouseOver); | ||
| document.removeEventListener('pointerout', this.onMouseOut); | ||
| this.removeTooltip(); | ||
| document.body.removeEventListener("pointerover", this.onOver); |
| this.element = wrapper.firstElementChild; | ||
|
|
||
| this.subElements = this.getSubElements(); | ||
| await this.initComponents(); |
There was a problem hiding this comment.
так лучше не делать!
попробуйте использовать тот подход к-й мы применяли для ColumnChart
сначала рисуем структуру компонента, а позже обновляем ее данными
другими словами не блокируем render
|
|
||
| import fetchJson from '../../utils/fetch-json.js'; | ||
| //import fetchJson from "../../utils/fetch-json.js"; |
There was a problem hiding this comment.
не оставляйте закомментированный код, ведь кроме Вас никто не знает почему он закомментирован
| const getRange = () => { | ||
| const now = new Date(); | ||
| const to = new Date(); | ||
| const from = new Date(now.setMonth(now.getMonth() - 1)); |
|
|
||
| await this.initComponents(); | ||
| this.subElements.rangePicker.append(this.rangePicker.element); |
There was a problem hiding this comment.
было бы неплохо сделать вставку компонент динамически - через цикл
| } | ||
| destroy() { | ||
| this.element.removeEventListener('date-select', this.onDateSelect); | ||
| this.rangePicker.destroy(); |
There was a problem hiding this comment.
тут также было бы удобно через цикл вызвать метод destroy
|
|
||
| constructor(params = null) { | ||
| this.productId = params[1] | ||
| //console.log(params); |
There was a problem hiding this comment.
скажите "НЕТ" закомментированному коду
| const getRange = () => { | ||
| const now = new Date(); | ||
| const to = new Date(); | ||
| const from = new Date(now.setMonth(now.getMonth() - 1)); |
| sortable: true, | ||
| sortType: 'date', | ||
| formatValue: data => { | ||
| const month = ['янв','фев','мар','апр','май','июн','июл','авг','сен','окт','ноя','дек']; |
There was a problem hiding this comment.
подумайте как сгенерировать названия месяцев динамически, ведь может быть ситуация когда в приложение добавят поддержку других языков
| @@ -4,7 +4,7 @@ export default async function(path, match) { | |||
| main.classList.add('is-loading'); | |||
|
|
|||
| const { default: Page } = await import(/* webpackChunkName: "[request]" */`../pages/${path}/index.js`); | |||
| const page = new Page(); | |||
| const page = new Page(match); | |||
No description provided.