Closed adityagarg06 closed 7 months ago
This is good stuff but I think that I need to explain my // TODO: improve loading state
comment. We have a single state.loading
for the entire app. It doesn't tell us what is loading. In the long term we would want to know that this specific thing is loading, possibly with RTK query? As a short-term bandaid fix I put in the hasLoadedData
state and the more complicated check of const showLoader = loading && !hasLoadedData;
. This prevents a situation where the loader shows up due to some unrelated API call elsewhere on the page.
There was a situation that was previously occurring where the sketch list would go into a loading state when you clicked the "add to collection" dropdown, as the "Add to collection" modal has its own API call which sets the global state.loading
to true
. TBH the two components where you changed it are probably fine? Since those are the secondary API calls. But I was putting that check in on all the components that I refactored to be on the safe side.
I have reverted the changes made in the AddToCollectionList, AddToCollectionSketchList, and Legal. I will create a new PR to implement them using the RTK query. Also made the change that you requested in the project.test file. Thanks for the feedback.
Progress on #2042 Changes:
I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123