Skip to content
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

Channel page caching #1750

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Channel page caching #1750

merged 1 commit into from
Jul 11, 2018

Conversation

daovist
Copy link
Contributor

@daovist daovist commented Jul 10, 2018

Issue #1255

Moves <BusyIndicator> to title bar.

Refactors contentList to show "No content found" only after fetch is complete; null while fetching.

channel page fetching

There is still some chance that a user will click the wrong tile but the update is very fast and pretty smooth. The best way I see to prevent this is to disable navigation for ~1sec following the update. This could be done by adding a throttle property to the state along with corresponding actions, reducers, and a thunk in doNavigate, or migrating from thunks to middleware, both of which seem well beyond the scope of this issue.

Copy link

@neb-b neb-b left a comment

Choose a reason for hiding this comment

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

Two very small changes.

<span className="empty">{__('No content found.')}</span>
) : (
null
);
Copy link

Choose a reason for hiding this comment

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

maybe

...
) : !fetching && <span className="empty">{__('No content found.')}</span>;

To avoid the nested ternarys

<h1>{name}</h1>
<h1>
{name}
{fetching ? <BusyIndicator /> : null}
Copy link

Choose a reason for hiding this comment

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

{!fetching && <BusyIndicator />}

@neb-b
Copy link

neb-b commented Jul 11, 2018

This is great. So much smoother.

@daovist
Copy link
Contributor Author

daovist commented Jul 11, 2018

I also cleaned up types in <BusyIndicator>.

@neb-b neb-b merged commit 14dc5dd into master Jul 11, 2018
@neb-b neb-b deleted the channel-caching branch July 11, 2018 14:58
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