Skip to content

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 philipjohn added type: enhancement New feature or request and removed Status: reporter feedback labels Oct 10, 2018
@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

Labels

type: enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants