Skip to content

HT6#82

Open
Meranuxapb wants to merge 1 commit intoTa4i:masterfrom
Meranuxapb:HT6
Open

HT6#82
Meranuxapb wants to merge 1 commit intoTa4i:masterfrom
Meranuxapb:HT6

Conversation

@Meranuxapb
Copy link
Copy Markdown

No description provided.

@Meranuxapb
Copy link
Copy Markdown
Author

Баг с закрытием статьи пытался решить на уровне reducer, чтобы полный Article не перетирался, но не получилось.
Pagination вынес в отдельный reducer, чтобы можно было в дальнейшем грузить постранично Article, но идею не докрутил

Copy link
Copy Markdown
Owner

@Ta4i Ta4i left a comment

Choose a reason for hiding this comment

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

Ты хорошо справился! Проблема с загрузкой первого артикла лежит в другом месте

componentDidMount() {
const {loadArticle, article, id} = this.props
if (!article || (!article.text && !article.loading)) {
// Получилось как-то топорно
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.

Корень проблемы в другом месте. Это конечно тоже фикс)

componentDidUpdate() {
// Эту логику можно описать а action, проверив флаг
// или наличие комментариев в state
// где это лучше делать?
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.

Ты прав, на самом деле принимать решение про данные на основе данных лучше на уровне редакс (то есть в наших thunk экшенах)


//Тут не понимаю, в какой момент нужно создавать экземпляр ArticleRecord
// К ключу loading разве можно будет обратиться через точку, пока не отработает
// LOAD_ARTICLE + SUCCESS?
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.

в этом редюсере кажется все было ок

const { type, payload, response, error } = action

switch (type) {
case SET_CURRENT_PAGE:
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.

я так понимаю этот экшн тебе не нужен

pageSelector,
// Не совсем понял, как в селекторе обрабатывать кейс с отсутствием элемента
// Если .get(page) undefined
// Или я на уровне reducer что-то упустил?
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.

Говоря конкретно про этот случай -- тут все ок. Загрузится -- появятся элементы

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