-
Notifications
You must be signed in to change notification settings - Fork 35
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
junior course react #26
base: master
Are you sure you want to change the base?
Conversation
schurin
commented
Aug 10, 2020
- Добавлен списко товаров
- Фильтрация
- Redux не подключал для шаринга фильтров использовал Context
src/PriceFilter/index.jsx
Outdated
width: '6em' | ||
} | ||
|
||
class PriceFilter extends BaseComponent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Думаю уже нет смысла наследоваться от BaseComponent, эта часть была для понимания как sCU реализуется. Наследуйся от React.Component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мне самому инетресно пока, когда был рерндер
Давай оставим :)
src/PriceFilter/index.jsx
Outdated
class PriceFilter extends BaseComponent { | ||
|
||
handleControlChange(filterName, value) { | ||
setTimeout(() => this.props.onChangeFilter({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вся асинхронщина должнв быть в componentDidMount
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это задержка пользовательского ввода
Чтобы фильтрация не происходила мгновенно после ввода каждого символа
src/PriceFilter/index.jsx
Outdated
<input style={ inuputStyle } | ||
type="text" | ||
defaultValue={ minPrice || 0 } | ||
onChange={(event)=> this.handleControlChange('minPrice', parseInt(event.target.value, 10))}/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом в этом случае стрелки ок. Но это часто приводит в проблемам "лишних" рендеров тк функция анонимная каждый раз.
Нужно это иметь ввиду.
https://ru.reactjs.org/docs/faq-functions.html#arrow-function-in-render тут подробнее
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
обратил внимание, для реакта это повод перерисовать компонент, ввиду того, что ссылка изменилась
в конкретной ситуации передаём minPrice
как аргумент
src/UserFilters/index.jsx
Outdated
handleFilterChange(changes) { | ||
const { filterName, value } = changes; | ||
|
||
setTimeout(() => this.props.onChangeFilter({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вообще не очень понятно зачем тут таймаут
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нужна задержка по вводу пользователя
правда ей тут немного не метсто
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
уберу ка я это от сюда
|
||
import { logger } from 'csssr-school-utils'; | ||
|
||
class BaseComponent extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Больше нет смысла самому писать BaseCompoment, можно сразу во всех случаях наследоваться от React.Component и юзать нативный sCU
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
<div className={s.goodsName}>{title}</div> | ||
<div> | ||
{ | ||
range(maxRating).map(i => React.createElement(ratingComponent, { key: i, isFilled: i <= rating })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React.createElenent тут лишний, можно же просто передать <ratingComponent {...props}>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix