Code Review - Githubissues

kimrgrey commented 6 years ago

[x] https://github.com/savvinsergey/react-slider/blob/master/src/components/image-slider.react.js#L10 - т.к. используется babel, подумай, как можно переписать этот компонент без использования конструктора
[x] https://github.com/savvinsergey/react-slider/blob/master/src/components/image-slider.react.js#L16 - использование функций, объявленных в дочерних компонентах - плохая практика, так как нарушает инкапсуляцию. Подумай, как это можно переписать (cм. https://reactjs.org/docs/lifting-state-up.html)

SliderControls

[x] переменная нигде не используется - https://github.com/savvinsergey/react-slider/blob/f504beff77a2b1f3c9b3aa44e16f62183d074957/src/components/slider-controls.react.js#L3
[x] https://github.com/savvinsergey/react-slider/blob/master/src/components/slider-controls.react.js#L7 - данный компонент реализует только метод render. Попробуй упростить его (см. https://reactjs.org/docs/components-and-props.html)
[x] https://github.com/savvinsergey/react-slider/blob/master/src/components/slider-controls.react.js#L12 - попробуй переписать с использованием lodash. В чем его преимущество по отношению к стандартной библиотеке?

[x] https://github.com/savvinsergey/react-slider/blob/master/src/components/slider-row.react.js - этот компонент должен быть значительно проще.
[x] Используй component state вместо внешних переменных
[x] Все timeout и interval объявленные внутри компонента должны очищаться на componentWillUnmount
[x] попробуй в state компонента хранить только id слайда, а в render расчитывать сдвиг
[x] this.props.onRef(this) - исключи такой код (передавать ссылку на себя - плохая практика)

[x] добавить .lock file. Зачем нужно его хранить в системе контроля версий?
[x] инкапсулировать верстку слайдера в сам компонент слайдера (см. css modules / styled-components)

savvinsergey commented 6 years ago

В целом всё готово. Отрефакторил в соответствии пунктами выше

В чем его преимущество по отношению к стандартной библиотеке?

Lodash упрощает и ускоряет разработку

см. css modules / styled-components

Использовал только styled-components

Зачем нужно его хранить в системе контроля версий?

В lock файле храниться полный "срез" дерева зависимостей

kimrgrey commented 6 years ago

@savvinsergey отлично, увидел, обязательно посмотрю и отпишусь завтра 👍