av-store's People
Contributors
Watchers
av-store's Issues
Review #5 (Map)
Magic numbers, можно вынести в константы.
Отсутствует вызов функции конструктора + переменная названа не в camelCase
Переменные не используются нигде кроме цикла, можно объявить их внутри цикла
Можно переписать на обычную функцию, а не самовызывающуюся
Review #3 (Home)
Свойство margin-top не применяется к элементу, нужно увеличить специфичность селектора, чтобы оно работало
Код вводящий в заблуждение. В начале модуля вызывается метод logout. Смотря на этот код, я совершаю предположение, что ты при загрузке этого модуля (страницы с модулем) делаешь (зачем-то) выход из приложения. Посмотрев код функции logout я понимаю, что в этой функции просто вешается обработчик на кнопку logout.
- в каждой странице происходит вызов этой функции.
Можно выделать код, касающийся назначения обработчиков событий на кнопки в хедере в отдельный модуль header.js и подключать его ко всем страницам, где показывается хэдер. Это решение более надёжное и уменьшает кол-во дублируещегося кода.
- нужно переназвать функцию. Можно что-то типо
assignHandlerToLogoutButton
Review #2 (signin)
Папку /dist заливать не следует, но если это нужно, чтобы работали github-pages, то всё норм
Опечатка bower_components. В целом можно bower_components удалить, поскольку мы не пользуемся менеджером пакетов bower.
Лучше работу с localStorage вынести в отдельные функции. Если поменяется ключ users на какой-либо другой, то придётся менять по всему приложению.
Можно упростить проверку
if (localStorage.getItem('users')) { в данном случае любое значение, которое возвращает что пользователей нету (undefined, '', null) будет учтено
Опционально можно добавить reset.css или normalize.css
https://github.com/artyom-voitas/AV-Store/blob/main/src/styles/index.scss
Название функции должно содержать глагол в большинстве случаев, потому что функция выполняет действие.
В данном случае функция может называть getExistingUser
Тоже самое тут. Функцию можно назвать getExistingUsers
Неочевидный результат у функции, то объект, то -1. Можно воспользоваться методом find и возвращать результат работы этого метода - искомый user либо undefined
!Boolean(-1) это false
https://github.com/artyom-voitas/AV-Store/blob/b3b82b15f4e9f1beee6148c6b208e9f749446181/src/js/sign-in.js#L27
Сравнение с true можно опустить и оставить checkUserData(userEmailValue, userPasswordValue)
Про работу с localStorage писал выше, следует вынести в отдельные функции
Про такого типа проверку писал выше, можно упростить и улучшить до if(!status)
Review #4 (Clients)
Нужно создать папку js/api с файлом users.js и засунуть туда URL для запроса и саму функцию запроса
Делать запрос к API прямо в коде модуля плохо. Обычно запросы делаются в ответ на какое-то действие пользователя (нажатие кнопки, отправка формы).
https://learn.javascript.ru/onload-ondomcontentloaded
Можно навесить обработчик на событие DOMContentLoaded и вызывать запрос к API и обработчика этого события.
Если ты хочешь использовать промисы в fetchUsers не обязательно делать "принудительный" resolve. Можно просто вернуть промис из метода json
return res.json()
Большой callback, можно разбить на маленькие функции, которые выполняют небольшие задачи. К примеру, создание строки таблицы, вывод кол-ва мужчин, вывод кол-ва женщин и т.д.
Лучше будет сначала сделать все нужные операции со строкой таблицы в памяти, а потом готовую строку вставить в DOM, чтобы минимизировать кол-во обращений к DOM.
Перед коммитом следует проверять убрал ли ты все console.log , который использовал для отладки и т.п.
Лучше не пилить свои костыли в попытках отформатировать дату. Используй toLocaleDateString метод у объекта Date
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleDateString
Лучше выделить в отдельную функцию, которую можно будет переиспользовать в других местах. К примеру extractRegistrationDateFromUser. Если изменится формат поля registered, то нужно будет поменять только функцию, а не искать все места в коде, где происходит корректное извлечение и трансформации даты.
Следуюет избегать в коде использования magic numbers. В данном случае 10
https://stackoverflow.com/questions/47882/what-is-a-magic-number-and-why-is-it-bad
Почти всю логику можно уместить в createConfirmationModal, чтобы не размазывать её по нескольким местам. Если тебе нужно сделать что-то специфичное на cancel или delete можно в функцию createConfirmationModal передать 2 коллбэка, onCancel, onConfirm
const onConfirm = () => {
deleteRowButton.parentElement.remove();
notification('Client successfully deleted!');
}
const onCancel = () => {
}
createConfirmationModal({ onCancel, onConfirm });
function createConfirmationModal({ onCancel, onConfirm }) {
// ....
let deleteButton = createSpecifiedDomElement('button', 'btn', 'Delete');
deleteButton.classList.add('btn-danger', 'btn__delete')
deleteButton.addEventListener('click', function() {
onConfirm()
// ... общая логика для delete кнопки
})
let cancelButton = createSpecifiedDomElement('button', 'btn', 'Cancel');
cancelButton.classList.add('btn-secondary', 'btn__cancel');
cancelButton.addEventListener('click', function() {
onCancel()
// ... общая логика для cancel кнопки
})
// ...
}
Неиспользуемая переменная
Название функции без глагола. Смотря на такое название сразу возникает вопрос: "Что notification? Всплывает, удаляется, появляется через время, появляется с задержкой". Банально можно назвать showNotification
Нужно вынести в отдельную функцию,
Review
index.html
всё чётко классно.
sign-in.js
В контексте твоего приложения index.js не лучшее место для хранения функций утилит. Будет лучше создать отдельный файл utils.js и туда вынести все фунции хелперы. index.js в основном является точкой входа в приложении, главным файлом для компонентов или файлом для реэскпорта нужного кода. К примеру есть следующая структура файлов для компонента Select:
/Select
/Select/Select.js
/Select/utils.js
/Select/constants.js
/Select/styles.js
/Select/index.js
Файл index.js тут имел примерно следующее содержание
import Select from './Select.js'
export default Select
Код, который собирается использовать Select может обраться просто к папке Select (из-за особенностей работы модулей в JS), а не к конкретному файлу в этой папке.
Функция, которая начинается с is предполагает проверку и возврат результата в виде Boolean. Пример:
function isStringEmpty(str) { return str.length !== 0 }. У тебя функция isFormFilled реализует логику дизейбла кнопки логина, нужно переназвать функцию. Я бы назвал disableFormButton.
Похожая ситуация тут, название не соответствует содержанию. Функция делает не проверку а манипуляцию с классами.
let isInputFieldValid = function (inputField, classOfInput) {
Названием для аргумента classOfInput тоже не подходящее, т.к. по факту ты передаёшь селектор, а не класс. Если кто-то другой будет пользоваться этим кодом, то он передаст "input-email" вместо ".input-email"
Можно использовать что-нибудь такое toggleInputWarning
Можно опустить явное обращение к window и оставить просто localStorage.getItem('email');
Излишнее сравнение с явным Boolean значением isUserExist(userEmail) === false. Можно сократить до !isUserExist(userEmail)
Можно передавать только значение value, а не весь инпут. Таким образом ты разделишь логику работы с данными и логику работы с DOM.
Проверка isEmailExistInLocalStorage() === userEmail сама по себе вовзращает Boolean значение, поэтому можно возвращать её сразу из функции.
- у тебя нету else блока, поэтому в текущей версии если проверка возвращает
false, то из функции возвращаетсяundefined
Сокращённая версия:
let isUserExist = function (email) {
let userEmail = email.value;
return isEmailExistInLocalStorage() === userEmail
}
=> if(loginToAccount())
loginToAccount по названию функции ожидается, что она "логинит" пользователя в приложение, по факту она проверяет есть ли пользователь с введёнными данными в приложении.
Можно передаывать не инпут, а просто значение пароля
Нейминг функции не соответствует логике
Предполагаю что у тебя будет определённый набор статусов пользователей, поэтому их можно вынести в отдельный файл с константами. Сами статусы будут выглядеть следующим образом
export const UserLoginStatus = {
logged: 'logged',
nonLogged: 'non-logged'
}
А вообще вместо статуса, ты можешь в localStorage заносить сам объект залогиненного пользователя, а не его статус.
setTimeout принимает 2 аргумента, без второго не совсем очевидно когда отработает переданный в него callback
window.location.replace('./home.html');
Несколько раз в коде встречалась такое выражение, если измениться название страницы, то тебе придётся искать это во всех местах в коде. Можно сделать функцию redirectToHome которая будет выполнять логику перенаправления на Home страницу.
В целом, старайся разделять работу с DOM и с данными. Функции делай в большей степени такие, которые работаю с данными. + почитай про SRP
https://melevir.medium.com/%D0%BA%D0%BE%D1%80%D0%BE%D1%87%D0%B5-%D0%B3%D0%BE%D0%B2%D0%BE%D1%80%D1%8F-%D0%BF%D1%80%D0%B8%D0%BD%D1%86%D0%B8%D0%BF-%D0%B5%D0%B4%D0%B8%D0%BD%D0%BE%D0%B9-%D0%BE%D1%82%D0%B2%D0%B5%D1%82%D1%81%D0%B2%D0%B5%D0%BD%D0%BD%D0%BE%D1%81%D1%82%D0%B8-92840ac55baa
Review #2 (signup)
Все комментарии для signin актуальны и тут
Опечатка passwordsDoNotMatch
Название переменное не очень удачное, лучше было бы confirmPasswordField
Вот тут пошла логика, которую лучше вынести в отдельную функции, чтобы не миксовать с кодом, который управляет разметкой
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.