31Vector31 / react-training

Hooks, Axios, HOC, Redux, Context, Reselect, Router.
0 stars 0 forks source link

review phone-book #8

Closed shevchuknine closed 2 years ago

shevchuknine commented 2 years ago

необходимо сделать приложение адаптивным, чтобы оно подстраивалось под ширину 1024px. и слегка увеличить контент: сейчас по кнопкам нужно целиться, к текст всматриваться чтобы прочитать https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/APIRequests.js#L24 чтобы не дублировать везде передачу конфигов, baseURL и т.д., можно создать инстанс axios-а и использовать его в запросах - axios.create https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/APIRequests.js#L15-L20 при одинаковых значениях ключей и переменных лучше использовать сокращенную форму {table, record: {name, surname, ...}} https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/PhoneBook/PhoneBook.js#L18 чтобы не возиться с этими вычислениями для тестовых проектов, можно использовать getTime метод у даты https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/PhoneBook/PhoneBook.js#L19 ты уже подписался на изменение contacts, можно не использовать функциональную форму для обновления https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/ContactsList/ContactsList.js#L31-L35 этот попап у тебя используется в двух местах, потому имеет смысл его рендерить на верхнем уровне и переспользовать между вызовами. функционал по созданию портала необходимо поместить внутрь попапа, иначе зачем его каждрый раз дублировать? рендерить себя в нужное место - функциональность попапа https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/ContactInfo/ContactInfo.js#L17 поскольку здесь всегда передается null, можно передать уже готовый колбек, сформированный на уровне выше https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/PopupForm/PopupForm.js#L58 дефолтное значение ("") определяется в здесь и в эффекте, потому лучше переиспользовать его (вынести в переменную) https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/PopupForm/PopupForm.js#L34 такая условная форма приритетна для использования если нужно что-то присвоить, например, в переменную. когда речь идет о вызове функций, лучше использовать if-else. ты можешь получить информацию о роли попапа (создание/редактирование) на основании contact, проп isEdit можно убрать https://github.com/31Vector31/react-training/blob/e088db5859078f4b4fd224d2e6fec82646458ff2/src/PhoneBook/PopupForm/PopupForm.js#L7-L9 чтобы неделать подобные вычисления, приоритетнее использовать функциональную форму инициализации состояния

shevchuknine commented 2 years ago

max-width на body, popup должен занимать максимальную ширину блока, а не окна функционал по созданию портала внутрь попапа - это его задача рендерить себя в нужное место

поскольку здесь всегда передается null, можно передать уже готовый колбек, сформированный на уровне выше

у тебя "висящие" колбеки по всему проекту, прочитав замечание в одном месте - примени его ко всему проекту https://github.com/31Vector31/react-training/blob/2cb8b42d0d149dbab6446059dcdd2affa459710c/src/PhoneBook/PhoneBook/PhoneBook.js#L26 ты же можешь достать id из первого аргумента, незачем дублировать аргументы https://github.com/31Vector31/react-training/blob/2cb8b42d0d149dbab6446059dcdd2affa459710c/src/PhoneBook/PhoneBook/PhoneBook.js#L45 необходимо мемоизировать эти вычисления _ https://github.com/31Vector31/react-training/blob/2cb8b42d0d149dbab6446059dcdd2affa459710c/src/PhoneBook/PopupConfirm/PopupConfirm.js#L13 deleteContact и так уже знает про id-шку, которую ему нужно передать, ты определяешь это на первом уровне

shevchuknine commented 2 years ago

контент должен быть центрирован, попап не должен вылазить на пределы ограниченных 1024 пикселей. ограничение ширины в body затронула другие проекты, нужно сделать это динамически _ https://github.com/31Vector31/react-training/blob/992314c5a1c2f4d98db0007ef8b144eddd13eb4a/src/PhoneBook/PhoneBook/PhoneBook.js#L53 https://github.com/31Vector31/react-training/blob/992314c5a1c2f4d98db0007ef8b144eddd13eb4a/src/PhoneBook/PhoneBook/PhoneBook.js#L58-L60 https://github.com/31Vector31/react-training/blob/992314c5a1c2f4d98db0007ef8b144eddd13eb4a/src/PhoneBook/PhoneBook/PhoneBook.js#L67 висящие колбеки, некоторые еще и дублируются. оборачивай все такие кейсы в useCallback