Skip to content

create comment route only HT6.1#60

Open
yarickhr wants to merge 1 commit into
romabelka:masterfrom
yarickhr:hryhoruk-home-task-6
Open

create comment route only HT6.1#60
yarickhr wants to merge 1 commit into
romabelka:masterfrom
yarickhr:hryhoruk-home-task-6

Conversation

@yarickhr
Copy link
Copy Markdown

Пока сделал только первую часть домашенего задания

Copy link
Copy Markdown
Owner

@romabelka romabelka left a comment

Choose a reason for hiding this comment

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

по-моему ты что-то не то реализовал

this.props.article.comments.map(id =>
<li key = {id} className = "test__comment-list--item">
<Comment id = {id}/>
<Comment id = {id} comment={comments.get(id)}/>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

У тебя Comment сам это умеет делать

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Хотел сделать чтобы этот компонент работал как в Article (где комент берется по id), так и отдельно (когда нужно с общего списка комментов брать комент (по его номеру)), поэтому передаю соmment с родителя

const commentSelector = oneCommentSelector()

return (state, ownProps) => ({
comment: commentSelector(state, ownProps.match.params)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Зачем это?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Чтобы взять коммент по его номеру (1, 2, 3, ...) c общего списка комментов

Copy link
Copy Markdown
Author

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