Commit 7ee719ab authored by Lyza Danger Gardner's avatar Lyza Danger Gardner

Change `isLoading` state to include if annotations have ever been fetched

For the places in the application that we care about whether “things are
currently loading”, whether annotations have ever been fetched is relevant.
Consider the application to be loading if there are any in-flight API
requests OR annotations have never been successfully loaded.
parent ea35d0e5
...@@ -33,9 +33,7 @@ function SidebarContent({ ...@@ -33,9 +33,7 @@ function SidebarContent({
const focusedGroupId = useStore(store => store.focusedGroupId()); const focusedGroupId = useStore(store => store.focusedGroupId());
const hasAppliedFilter = useStore(store => store.hasAppliedFilter()); const hasAppliedFilter = useStore(store => store.hasAppliedFilter());
const isFocusedMode = useStore(store => store.focusModeEnabled()); const isFocusedMode = useStore(store => store.focusModeEnabled());
const annotationsLoading = useStore(store => { const isLoading = useStore(store => store.isLoading());
return !store.hasFetchedAnnotations() || store.isFetchingAnnotations();
});
const isLoggedIn = useStore(store => store.isLoggedIn()); const isLoggedIn = useStore(store => store.isLoggedIn());
const linkedAnnotationId = useStore(store => const linkedAnnotationId = useStore(store =>
store.directLinkedAnnotationId() store.directLinkedAnnotationId()
...@@ -68,7 +66,7 @@ function SidebarContent({ ...@@ -68,7 +66,7 @@ function SidebarContent({
// If, after loading completes, no `linkedAnnotation` object is present when // If, after loading completes, no `linkedAnnotation` object is present when
// a `linkedAnnotationId` is set, that indicates an error // a `linkedAnnotationId` is set, that indicates an error
const hasDirectLinkedAnnotationError = const hasDirectLinkedAnnotationError =
!annotationsLoading && linkedAnnotationId ? !linkedAnnotation : false; !isLoading && linkedAnnotationId ? !linkedAnnotation : false;
const hasDirectLinkedGroupError = useStore(store => const hasDirectLinkedGroupError = useStore(store =>
store.directLinkedGroupFetchFailed() store.directLinkedGroupFetchFailed()
...@@ -78,7 +76,7 @@ function SidebarContent({ ...@@ -78,7 +76,7 @@ function SidebarContent({
hasDirectLinkedAnnotationError || hasDirectLinkedGroupError; hasDirectLinkedAnnotationError || hasDirectLinkedGroupError;
const showTabs = !hasContentError && !hasAppliedFilter; const showTabs = !hasContentError && !hasAppliedFilter;
const showSearchStatus = !hasContentError && !annotationsLoading; const showSearchStatus = !hasContentError && !isLoading;
// Show a CTA to log in if successfully viewing a direct-linked annotation // Show a CTA to log in if successfully viewing a direct-linked annotation
// and not logged in // and not logged in
...@@ -86,7 +84,7 @@ function SidebarContent({ ...@@ -86,7 +84,7 @@ function SidebarContent({
linkedAnnotationId && linkedAnnotationId &&
!isLoggedIn && !isLoggedIn &&
!hasDirectLinkedAnnotationError && !hasDirectLinkedAnnotationError &&
!annotationsLoading; !isLoading;
const prevGroupId = useRef(focusedGroupId); const prevGroupId = useRef(focusedGroupId);
...@@ -136,7 +134,7 @@ function SidebarContent({ ...@@ -136,7 +134,7 @@ function SidebarContent({
{hasDirectLinkedGroupError && ( {hasDirectLinkedGroupError && (
<SidebarContentError errorType="group" onLoginRequest={onLogin} /> <SidebarContentError errorType="group" onLoginRequest={onLogin} />
)} )}
{showTabs && <SelectionTabs isLoading={annotationsLoading} />} {showTabs && <SelectionTabs isLoading={isLoading} />}
{showSearchStatus && <SearchStatusBar />} {showSearchStatus && <SearchStatusBar />}
<ThreadList thread={rootThread} /> <ThreadList thread={rootThread} />
{showLoggedOutMessage && <LoggedOutMessage onLogin={onLogin} />} {showLoggedOutMessage && <LoggedOutMessage onLogin={onLogin} />}
......
...@@ -56,7 +56,7 @@ describe('SidebarContent', () => { ...@@ -56,7 +56,7 @@ describe('SidebarContent', () => {
hasAppliedFilter: sinon.stub(), hasAppliedFilter: sinon.stub(),
hasFetchedAnnotations: sinon.stub(), hasFetchedAnnotations: sinon.stub(),
hasSidebarOpened: sinon.stub(), hasSidebarOpened: sinon.stub(),
isFetchingAnnotations: sinon.stub(), isLoading: sinon.stub().returns(false),
isLoggedIn: sinon.stub(), isLoggedIn: sinon.stub(),
getState: sinon.stub(), getState: sinon.stub(),
profile: sinon.stub().returns({ userid: null }), profile: sinon.stub().returns({ userid: null }),
...@@ -113,8 +113,7 @@ describe('SidebarContent', () => { ...@@ -113,8 +113,7 @@ describe('SidebarContent', () => {
context('when viewing a direct-linked annotation', () => { context('when viewing a direct-linked annotation', () => {
context('successful direct-linked annotation', () => { context('successful direct-linked annotation', () => {
beforeEach(() => { beforeEach(() => {
fakeStore.hasFetchedAnnotations.returns(true); fakeStore.isLoading.returns(false);
fakeStore.isFetchingAnnotations.returns(false);
fakeStore.annotationExists.withArgs('someId').returns(true); fakeStore.annotationExists.withArgs('someId').returns(true);
fakeStore.directLinkedAnnotationId.returns('someId'); fakeStore.directLinkedAnnotationId.returns('someId');
fakeStore.findAnnotationByID fakeStore.findAnnotationByID
...@@ -149,8 +148,7 @@ describe('SidebarContent', () => { ...@@ -149,8 +148,7 @@ describe('SidebarContent', () => {
context('error on direct-linked annotation', () => { context('error on direct-linked annotation', () => {
beforeEach(() => { beforeEach(() => {
// This puts us into a "direct-linked annotation" state // This puts us into a "direct-linked annotation" state
fakeStore.hasFetchedAnnotations.returns(true); fakeStore.isLoading.returns(false);
fakeStore.isFetchingAnnotations.returns(false);
fakeStore.directLinkedAnnotationId.returns('someId'); fakeStore.directLinkedAnnotationId.returns('someId');
// This puts us into an error state // This puts us into an error state
...@@ -179,8 +177,7 @@ describe('SidebarContent', () => { ...@@ -179,8 +177,7 @@ describe('SidebarContent', () => {
context('error with direct-linked group', () => { context('error with direct-linked group', () => {
beforeEach(() => { beforeEach(() => {
fakeStore.hasFetchedAnnotations.returns(true); fakeStore.isLoading.returns(false);
fakeStore.isFetchingAnnotations.returns(false);
fakeStore.directLinkedGroupFetchFailed.returns(true); fakeStore.directLinkedGroupFetchFailed.returns(true);
}); });
...@@ -229,7 +226,7 @@ describe('SidebarContent', () => { ...@@ -229,7 +226,7 @@ describe('SidebarContent', () => {
it('renders search status', () => { it('renders search status', () => {
fakeStore.hasFetchedAnnotations.returns(true); fakeStore.hasFetchedAnnotations.returns(true);
fakeStore.isFetchingAnnotations.returns(false); fakeStore.isLoading.returns(false);
const wrapper = createComponent(); const wrapper = createComponent();
...@@ -237,7 +234,7 @@ describe('SidebarContent', () => { ...@@ -237,7 +234,7 @@ describe('SidebarContent', () => {
}); });
it('does not render search status if annotations are loading', () => { it('does not render search status if annotations are loading', () => {
fakeStore.hasFetchedAnnotations.returns(false); fakeStore.isLoading.returns(true);
const wrapper = createComponent(); const wrapper = createComponent();
......
...@@ -145,10 +145,13 @@ function isFetchingAnnotations(state) { ...@@ -145,10 +145,13 @@ function isFetchingAnnotations(state) {
/** /**
* Return true when any activity is happening in the app that needs to complete * Return true when any activity is happening in the app that needs to complete
* before the UI will be idle. * before the UI is ready for interactivity with annotations.
*/ */
function isLoading(state) { function isLoading(state) {
return state.activity.activeApiRequests > 0; return (
state.activity.activeApiRequests > 0 ||
!state.activity.hasFetchedAnnotations
);
} }
/** /**
......
...@@ -26,8 +26,8 @@ describe('sidebar/store/modules/activity', () => { ...@@ -26,8 +26,8 @@ describe('sidebar/store/modules/activity', () => {
}); });
describe('#isLoading', () => { describe('#isLoading', () => {
it('returns false with the initial state', () => { it('returns true when annotations have never been loaded', () => {
assert.equal(store.isLoading(), false); assert.isTrue(store.isLoading());
}); });
it('returns true when API requests are in flight', () => { it('returns true when API requests are in flight', () => {
...@@ -35,14 +35,16 @@ describe('sidebar/store/modules/activity', () => { ...@@ -35,14 +35,16 @@ describe('sidebar/store/modules/activity', () => {
assert.equal(store.isLoading(), true); assert.equal(store.isLoading(), true);
}); });
it('returns false when all requests end', () => { it('returns false when all requests end and annotations are fetched', () => {
store.apiRequestStarted(); store.apiRequestStarted();
store.apiRequestStarted(); store.apiRequestStarted();
store.apiRequestFinished(); store.apiRequestFinished();
store.annotationFetchStarted();
assert.equal(store.isLoading(), true); assert.equal(store.isLoading(), true);
store.apiRequestFinished(); store.apiRequestFinished();
store.annotationFetchFinished();
assert.equal(store.isLoading(), false); assert.equal(store.isLoading(), false);
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment