kristof22122 / Ex2-React-Program1

0 stars 0 forks source link

review picture-galery #6

Closed shevchuknine closed 2 years ago

shevchuknine commented 2 years ago

https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/App.js#L12 реализовать debounce можно без этого состояния, оно излишне https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/App.js#L31 когда передаешь методы в элементы, а не в компоненты, следить за имутабельностью нет необходимости - это не приводит к улучшениям производительности https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/App.js#L42-L47 зачем здесь try/catch? какой вид эксепшена ты отлавливаешь? вызов setPictures можно сократить до одной строчки промисы для организации асинхронного кода https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/api.js#L16 формировать параметры можно с помощью config.params, axios берет на себя все конкатенации https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/components/Pictures/Pictures.js#L11-L12 необходимость открытия модалки можно определять по одному состоянию - showPictureUrl https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/components/Pictures/Pictures.js#L20 деструктуризация _ https://github.com/kristof22122/Ex2-React-Program1/blob/335ae9fc175808fbb5cfb818ebc0888062d61089/picture-gallery/src/components/Modal/Modal.js#L20 имеет смысл рендерить null, т.е. ничего, в случае, если модалка закрыта

shevchuknine commented 2 years ago

https://github.com/kristof22122/Ex2-React-Program1/blob/7be47b7d10f38e48d89ea424adc4172dc6a65f79/picture-gallery/src/App.js#L64 форма должна оставаться контроллируемой при реализации debounce. debounc-ить нужно вызов api _ https://github.com/kristof22122/Ex2-React-Program1/blob/7be47b7d10f38e48d89ea424adc4172dc6a65f79/picture-gallery/src/App.js#L40 в обоих случаях ты вызываешь одну и ту же функцию, значит условие можно перенести в параметр функции. конкатенация массивов через распыление

shevchuknine commented 2 years ago

https://github.com/kristof22122/Ex2-React-Program1/blob/d2535b4a65003557b46cb8f57075eaed2937e405/picture-gallery/src/App.js#L40 конкатенация массивов через распыление const array = [...oldArray, newElement] _ https://github.com/kristof22122/Ex2-React-Program1/blob/d2535b4a65003557b46cb8f57075eaed2937e405/picture-gallery/src/App.js#L15 сейчас у тебя запросы на даные летят при любом изменении поля ввода. это неправильная реализация. суть должна быть следующая: изменяется поле ввода, после этого в течении 500мс поле не меняется - отправляется запрос. реализовать это нужно с помощью оборачивания в debounce вызова апи