Skip to content

Add loading state to api actions #494

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

Closed
wants to merge 2 commits into from

Conversation

cain
Copy link
Contributor

@cain cain commented Jun 26, 2018

Added loading to some api actions

@philipjohn
Copy link
Contributor

Thanks for this - can you expand this PR with details about what issue this fixes with details reproduction steps please?

@cain
Copy link
Contributor Author

cain commented Oct 8, 2018

@philipjohn So in the state we hold a loading variable to handle various tasks while waiting for the api to return. This PR just expands loading state to the other redux api actions.

EG: Deleting/editing entries will now have a loading state to do conditionals off.

I'm not sure this is 100% neccessary if #527 is merged into master, since that PR uses a isPublishing state variable instead.

@cain
Copy link
Contributor Author

cain commented Oct 8, 2018

@davidsword is this PR neccessary or useless once your PR is into master :)

@philipjohn
Copy link
Contributor

You may be right about that, and I'm planning to test #527 and include it in v2.0 if all is well :)

@cain
Copy link
Contributor Author

cain commented Oct 10, 2018

Yeah i think that PR covers this. Would be good to get @davidsword's feedback

@davidsword
Copy link
Contributor

davidsword commented Oct 11, 2018

Hey! Sorry for the delay. This is a good idea and similar to what I was looking for before my PR. I like the naming 'loading' better.

I feel this lacks the specifics of what is being done, and to what. In the situation I needed something like this for, <Entry> and <EditorContainer>, I needed to know if it was submitting a new entry, or when updating/deleting, which entry ID it was being done for. It's really good to know that things are loading, but I think be more useful to know exactly what is loading.

@cain
Copy link
Contributor Author

cain commented Oct 11, 2018

Yeah i agree with @davidsword
Please feel free to open this back up if its necessary

@cain cain closed this Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants