Implement basic loading component #423
Conversation
jeremiak
force-pushed
the
jk-loading-state
branch
2 times, most recently
from
0462f16
to
8d5b58e
Compare
jeremiak
changed the title
|
This is ready for review @msecret |
|
@jeremiak This might be more for another PR / I'm missing something but I'm wondering does this account for a certain model type (e.g. users) doing multiple fetching at the same time on the page. I'm sure it doesn't happen now but just in case it happens in the future. I'm wondering if |
|
@jcscottiii it is a great point. Just to clarify, you're saying that if:
Therefore, the store is actually waiting for a request but it's Even if we were to keep track of each request going out, it would still be hard to know if every request an interface needed had come back with fresh data or if it was stale. Thoughts on how we might want to do this otherwise? |
|
let me think of it this way: instead let's do it this way make the fetching as part of the component instead of the store. thoughts @msecret @jeremiak ? |
|
According to our design, only the store can keep state, such as fetching. But the components can be listening to the store. |
|
@msecret ahh yeah. hmmm. so could each component get a way to know that the store is changing it's particular fetching state? |
|
Yea I mean, we coudl keep fetching in the stores but separate different fetching out more so each fetching is more specific? Then components could be more specific about what fetching they're waiting for? |
|
@msecret i got ya! |
|
If this works, we should use it for the time being, and get these merge conflicts fixed. @jcscottiii if you're interested in taking a look at improving the fetching tracking in the next few days as we just discussed, you definitely should, I think it could be a good improvement. |
This is a potential implementation for loading states. Adds a fetching attribute to the base store, which should always be a boolean value. Includes basic tests
Right now, it is hard to use a single `fetching` value on the store because a store might have a few different requests that have to be received before the full state can be constructed. For example, on the app page we need to not only get the app but also the app states and routes. If we only have one attribute on the store, then it is hard to time when the attribute gets set to false or true
assertAction() was not checking anything about the “type” variable, which means that if it was undefined the tests would pass.
Add event constant types to represent when we have multiple requests in flight for an app’s information and when they’ve all come back. We want to keep these in one action so that we can ensure that `.fetching` will be set appropriately on the AppStore.
We have two new action types to help us coordinate when we have multiple requests in flight. We want to know when all the requests have been made and when the responses have come back so we can set the `.fetching` attribute on the store appropriately. This will be used for showing loading states in the UI.
All actions are dispatched by action creators, so we need two new action creators to represent our two new appActionTypes.
Added a new function to the cfApi helper library called `fetchAllApp`, which takes an app guid. It then calls the `fetchApp` and the `fetchAppStats` functions, and once both promises have resolved, it calls the `receivedAppAll` app action.
Instead of making two individual requests for the app and the app stats, we’ll use the fetchAll app action creator to set the `fetching` value of AppStore to `true`, initiate the HTTP requests and then switch `fetching` back to `false` once both promises have resolved.
This component is to be used as a loading component, and takes a text prop and displays it.
Use the <Loading /> component in the <AppPage /> to display some basic loading text while the requests are being made. This will allow us to have a display for the following states: * Fetch in progress * Fetch returned with no results * Fetch returned with results
Changes the OrgStore so that the `.fetching` attribute is set appropriately.
Set the fetching attribute on the SpaceStore depending on the action type. Modifies the tests to properly test the behavior, while relying on the BaseStore specs to test the inherited function behavior
Use a spaceAction to trigger the cfApi.fetchSpace call so that the store can be notified of a request being made and update the fetching attribute appropriately.
Leverage the Loading component in the AppList component so that the user can understand if there are no apps, the requests are inflight, or if the request has already come back.
Set value of `.fetching` in UserStore depending on which action types. Adjust tests to pass and make sure the new behavior is tested.
Show loading component when looking at the Users in a space
Adds behavior to set `.fetching` on the ServiceInstanceStore depending on the serviceAction type. Change tests to test this new behavior
Add the loading component to the service instance list so that users can have a better understanding if the app is: 1. Requesting data from the server 2. Has received data from the server 3. There are no services in the space
* Sets fetching to true on SERVICES_FETCH * Sets fetching to false on SERVICES_RECEIVED Change tests to capture this behavior
To ensure that the stores set their respective `.fetching` value appropriately, we should use actions to trigger API requests instead of calling the API directly
The marketplace component relies on four different stores, so it will show the loading component if any of them are currently fetching data from the server.
jeremiak
force-pushed
the
jk-loading-state
branch
from
8d5b58e
to
5bf0872
Compare
|
@msecret @jcscottiii I fixed the merge conflicts so this should be ready for review now |
|
|
||
| import React from 'react'; | ||
|
|
||
| const propTypes = { |
There was a problem hiding this comment.
Is there a reason you defined these into separate variables rather then directly like other components:
Loading.proptTypes = {
text
...
Is this a better pattern we should adopt?
Changes the stores to have a
.fetchingattribute, which will be a boolean. How the stores implement the toggling of this value is up to each individual store. Then the components can check for requests that are currently in flight and use the<Loading />component to display a standardized loading message.We still need to design the loading component, as it is pretty stark right now but it is a first pass.
Refs #383