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

Can't tell whether isLoading is caused by auto-refresh or call to refresh #4

Open
atsnisov opened this issue May 1, 2018 · 2 comments

Comments

@atsnisov
Copy link

atsnisov commented May 1, 2018

I can't figure out a way to implement the following use case with redux-autoloader:
There's a reduxAutoloader wrapped component with auto-refresh enabled and it's utilising the refresh prop. The wrapped component should render loading spinner when the data is being reloaded as result of refresh prop but should not render the spinner when the data is loading as result of auto-refresh interval.

I can't figure out how the wrapped component could separate the two cases when looking at the props redux-autoloader is passing.

I also think the terms refresh and auto-refresh are mixed in the documentation which causes confusion. For instance, I incorrectly thought isRefreshing means someone has called refresh and the data is now being refreshed because of that.

@nygardk
Copy link
Collaborator

nygardk commented May 6, 2018

You are very right, the terms are confusing. It's clearly an oversight from me.

The first thing that probably should be done would be to remove the confusion between "reload" and "refresh" as they mean essentially the same. However, if "autoRefreshing" was called "autoReloading", it would be misleading as there could still be automatic reloads based on other options (reload or reinitialize). Thus, I think autoRefreshing should be simply renamed to polling.

Let me try to make more sense out of the naming by virtually renaming some of the options and props:

options

original name new name
autoRefreshInterval pollInterval

props

original name new name
isRefreshing isPolling
refresh() reload()
startAutoRefresh() startPolling()
stopAutoRefresh() stopPolling()

I can't figure out how the wrapped component could separate the two cases when looking at the props redux-autoloader is passing.

From the new prop names it might be more clear that the library doesn't currently keep state of how the loading (->isLoading) was triggered. With this implementation it is not possible to know the source; whether it was because of polling, manual reload or a lifecycle reload.

Clearly the isRefreshing is the most misleading here, as it should had originally been named as isAutoRefreshing.

@atsnisov
Copy link
Author

atsnisov commented May 7, 2018

Great suggestion for the name changes imo!

However, the main reason I opened the ticket was that I needed (and I still need) a way to distinguish the different reasons for the loading. I have a view where I use polling to keep the data fresh but the data in the view could also change due to a user-invoked async operation, in which case I want to show a loader until the data is up-to-date again. Could you add support for that?

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

No branches or pull requests

2 participants