-
Notifications
You must be signed in to change notification settings - Fork 13
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
HT4 by Balioura #38
base: master
Are you sure you want to change the base?
HT4 by Balioura #38
Conversation
}; | ||
|
||
export default connect((state) => ({ | ||
allReviews: state.reviews, |
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.
используйте селекторы
|
||
const AverageRate = ({ reviews, allReviews }) => { | ||
|
||
const averageRating = useMemo(() => { |
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.
все эти вычисления нужно выносить в селекторы, чтобы компонент занимался только рендером
@@ -5,7 +5,7 @@ import Restaurant from '../restaurant'; | |||
import Tabs from '../tabs'; | |||
|
|||
const Restaurants = ({ restaurants }) => { | |||
const tabs = restaurants.map((restaurant) => ({ | |||
const tabs = Object.values(restaurants).map(restaurant => ({ |
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.
это тоже должно быть в селекторах и мемоизировано
|
||
export const increment = (id) => ({ type: INCREMENT, payload: { id } }); | ||
export const decrement = (id) => ({ type: DECREMENT, payload: { id } }); | ||
export const remove = (id) => ({ type: REMOVE, payload: { id } }); | ||
export const addReview = (item) => ({ type: ADD_REVIEW, payload: item }); |
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.
тут можно (и нужно) обойтись одним екшеном, т.к. все редьюсеры реагирую на все екшены, я на занятии расскажу в чем проблемы такого подхода
|
||
if (action.type === 'ADD_REVIEW' || action.type === 'ADD_USER') { | ||
const uuid = uuidv4(); | ||
action.payload = { |
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.
не стоит мутировать action, лучше создать новый
<div className={styles.review} data-id="review"> | ||
<div className={styles.content}> | ||
<div> | ||
<h4 className={styles.name} data-id="review-user"> | ||
{user} | ||
{userName} {/* // это лучше отдельным компонентом? */} |
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.
нет, зачем?
Review.defaultProps = { | ||
user: 'Anonymous', | ||
const mapStateToProps = (state, ownProps) => { | ||
const review = state.reviews[ownProps.id]; |
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.
все это нужно выносить в селекторы, чтобы компонент занимался только рендером
No description provided.