Skip to content

HT5#77

Open
omiceron wants to merge 7 commits intoromabelka:masterfrom
omiceron:HT5
Open

HT5#77
omiceron wants to merge 7 commits intoromabelka:masterfrom
omiceron:HT5

Conversation

@omiceron
Copy link

@omiceron omiceron commented Feb 4, 2018

No description provided.

isOpen={article.id === this.state.openItemId}
toggleOpen={this.toggleOpenItemMemoized(article.id)}
const articleElements = articles.map(({id}) => <li key={id}>
<Article id={id}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не понял зачем это

value: article.id
// console.log(articles)

const options = Object.values(articles).map(({title, id}) => ({
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше селектор, который достанет из в виде массива

// можно добавить проверку на id, чтобы не было повторений, но это потом

export default store => next => action => {
if (action.type === ADD_COMMENT) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

через мидлвары будет проходить каждый экшин, они должны быть максимально общими, завязывать на конкретные экшины - не лучшее решение


export default store => next => action => {
if (action.type === ADD_COMMENT) {
action.payload.id = generateRandomId();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше не мутировать payload, мало-ли что там станут передавать


case ADD_COMMENT:
const targetArticle = {...articlesState[payload.targetArticle]};
targetArticle.comments = [...targetArticle.comments, payload.id];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments может быть undefined


const enhancer = composeEnhancers(
applyMiddleware(logger)
applyMiddleware(logger, randomizer)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше логгер в конце ставить

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants