Skip to content
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

Final project #105

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

DmitriyBukreev
Copy link

No description provided.

import SortableList from '../sortable-list';
import NotificationMessage from '../notification';

const BACKEND_URL = process.env.BACKEND_URL;
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good
лайк за переменные окружения

categoryURL = new URL('/api/rest/categories?_sort=weight&_refs=subcategory', BACKEND_URL);
subcategoryURL = new URL('/api/rest/subcategories', BACKEND_URL);

onOpen = event => {
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

this.element.addEventListener('list-updated', this.onListUpdate);
}

async loadData() {
Copy link
Owner

Choose a reason for hiding this comment

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

тут не нужен async

@@ -1,89 +1,123 @@
import fetchJson from '../../utils/fetch-json.js';

const BACKEND_URL = process.env.BACKEND_URL;
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍

update ({headerData, bodyData}) {
this.subElements.header.textContent = headerData;
this.subElements.body.innerHTML = this.getColumnBody(bodyData);
async loadData(from, to) {
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍

@@ -0,0 +1,99 @@
import DoubleSlider from '../double-slider/index.js';

export default class ProductFilter {
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍
лайк за отделение логики в собственный компонент


const IMGUR_CLIENT_ID = process.env.IMGUR_CLIENT_ID;

export default class ImageUploader {
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

onUpload = async () => {
try {
const [file] = this.input.files;
if (!file) return;
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

return wrapper.firstElementChild;
}

async upload(file) {
Copy link
Owner

@boris-catsvill boris-catsvill Apr 21, 2023

Choose a reason for hiding this comment

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

👍 good

onSubmit = event => {
event.preventDefault();
onImageUploaded = event => {
const listElement = this.getImageListItem(event.detail.image);
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

};
async render() {
const categoriesPromise = this.loadCategories();
const productPromise = this.productId.length
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

const element = productForm.querySelector(`#${item}`);

element.value = this.formData[item] || this.defaultFormData[item];
if (!this.productId.length) elements.subcategory.selectedIndex = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

не экономьте на скобках в блоках if

for (const category of categories) {
for (const subcategory of category.subcategories) {
options.push(
`<option value="${subcategory.id}">${category.title} > ${subcategory.title}</option>`
Copy link
Owner

Choose a reason for hiding this comment

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

обратите внимание на конструктор new Option

this.element.style.left = left + 'px';
this.element.style.top = top + 'px';
destroy() {
document.removeEventListener('pointerover', this.addTooltipHandler);
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍
альтернатива может быть в использовании AbortController

@@ -3,6 +3,14 @@ import tooltip from './components/tooltip/index.js';

tooltip.initialize();

const button = document.querySelector('.sidebar__toggler');
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍

@@ -14,4 +22,16 @@ router
.addRoute(/^categories$/, 'categories')
.addRoute(/^404\/?$/, 'error404')
.setNotFoundPagePath('error404')
.addRouteHandler(route => {
for (let item of sidebarList) {
Copy link
Owner

Choose a reason for hiding this comment

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

альтернатива:
выделить ссылки для навигации в отдельный компонент и внутри него описать логику подстановки active


destroy() {
this.remove();
for (const component of Object.values(this.components)) {
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍

const ordersDataTotal = ordersData.reduce((accum, item) => accum + item);
const salesDataTotal = salesData.reduce((accum, item) => accum + item);
const customersDataTotal = customersData.reduce((accum, item) => accum + item);
loadData(from, to) {
Copy link
Owner

Choose a reason for hiding this comment

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

good 👍

}

initComponents() {
const path = decodeURI(window.location.pathname);
Copy link
Owner

Choose a reason for hiding this comment

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

🔴 please don't do this
так делать нельзя, компонент не должен обращаться к window.location.
это дело роутера, и из роутера id товара должно попасть в данную страницу

initComponents() {
const now = new Date();
const to = new Date();
const from = new Date(now.setMonth(now.getMonth() - 1));
Copy link
Owner

Choose a reason for hiding this comment

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

👍 good

Copy link
Owner

@boris-catsvill boris-catsvill left a comment

Choose a reason for hiding this comment

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

👍 good
посмотрите комменты

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