Commit 631372ee authored by Robert Knight's avatar Robert Knight

Implement new progress display for import

Implement a new progress display based on the mocks in
https://github.com/hypothesis/client/issues/5740#issuecomment-1699585881.

Fixes https://github.com/hypothesis/client/issues/5740
parent 32294451
...@@ -8,8 +8,8 @@ import { readExportFile } from '../../helpers/import'; ...@@ -8,8 +8,8 @@ import { readExportFile } from '../../helpers/import';
import { withServices } from '../../service-context'; import { withServices } from '../../service-context';
import type { ImportAnnotationsService } from '../../services/import-annotations'; import type { ImportAnnotationsService } from '../../services/import-annotations';
import { useSidebarStore } from '../../store'; import { useSidebarStore } from '../../store';
import CircularProgress from '../CircularProgress';
import FileInput from './FileInput'; import FileInput from './FileInput';
import LoadingSpinner from './LoadingSpinner';
export type ImportAnnotationsProps = { export type ImportAnnotationsProps = {
importAnnotationsService: ImportAnnotationsService; importAnnotationsService: ImportAnnotationsService;
...@@ -125,7 +125,16 @@ function ImportAnnotations({ ...@@ -125,7 +125,16 @@ function ImportAnnotations({
const parseInProgress = file && !annotations && !error; const parseInProgress = file && !annotations && !error;
// True if we're validating or importing. // True if we're validating or importing.
const busy = parseInProgress || store.importsPending() > 0; const importsPending = store.importsPending();
const importsTotal = store.importsTotal();
const importsCompleted = importsTotal - importsPending;
const busy = parseInProgress || importsPending > 0;
const importProgress =
importsPending > 0
? Math.round((importsCompleted / importsTotal) * 100)
: null;
return ( return (
<> <>
...@@ -159,7 +168,6 @@ function ImportAnnotations({ ...@@ -159,7 +168,6 @@ function ImportAnnotations({
</Select> </Select>
</> </>
)} )}
{busy && <LoadingSpinner />}
{error && ( {error && (
// TODO - Add a support link here. // TODO - Add a support link here.
<p data-testid="error-info"> <p data-testid="error-info">
...@@ -167,12 +175,20 @@ function ImportAnnotations({ ...@@ -167,12 +175,20 @@ function ImportAnnotations({
</p> </p>
)} )}
<CardActions> <CardActions>
{importProgress !== null && (
<span data-testid="progress-text" className="text-grey-6">
{importProgress}% complete
</span>
)}
<Button <Button
data-testid="import-button" data-testid="import-button"
disabled={!importReady || busy} disabled={!importReady || busy}
onClick={importAnnotations} onClick={importAnnotations}
variant="primary" variant="primary"
> >
{importProgress !== null && (
<CircularProgress size={22} value={importProgress} />
)}
Import Import
</Button> </Button>
</CardActions> </CardActions>
......
...@@ -20,6 +20,7 @@ describe('ImportAnnotations', () => { ...@@ -20,6 +20,7 @@ describe('ImportAnnotations', () => {
defaultAuthority: sinon.stub().returns('example.com'), defaultAuthority: sinon.stub().returns('example.com'),
focusedGroup: sinon.stub().returns({ id: 'group-1' }), focusedGroup: sinon.stub().returns({ id: 'group-1' }),
importsPending: sinon.stub().returns(0), importsPending: sinon.stub().returns(0),
importsTotal: sinon.stub().returns(0),
isFeatureEnabled: sinon.stub().returns(true), isFeatureEnabled: sinon.stub().returns(true),
hasFetchedAnnotations: sinon.stub().returns(true), hasFetchedAnnotations: sinon.stub().returns(true),
isFetchingAnnotations: sinon.stub().returns(false), isFetchingAnnotations: sinon.stub().returns(false),
...@@ -293,15 +294,26 @@ describe('ImportAnnotations', () => { ...@@ -293,15 +294,26 @@ describe('ImportAnnotations', () => {
); );
}); });
it('shows loading spinner during import', () => { it('shows progress indicator during import', () => {
fakeStore.importsPending.returns(2); fakeStore.importsPending.returns(1);
fakeStore.importsTotal.returns(2);
const wrapper = createImportAnnotations(); const wrapper = createImportAnnotations();
assert.isTrue(wrapper.exists('LoadingSpinner'));
const progressText = wrapper.find('[data-testid="progress-text"]');
assert.isTrue(progressText.exists());
assert.equal(progressText.text(), '50% complete');
const progress = wrapper.find('CircularProgress');
assert.isTrue(progress.exists());
assert.equal(progress.prop('value'), 50);
fakeStore.importsPending.returns(0); fakeStore.importsPending.returns(0);
fakeStore.importsTotal.returns(0);
wrapper.setProps({}); // Force re-render wrapper.setProps({}); // Force re-render
assert.isFalse(wrapper.exists('LoadingSpinner')); assert.isFalse(wrapper.exists('CircularProgress'));
assert.isFalse(wrapper.exists('[data-testid="progress-text"]'));
}); });
it( it(
......
...@@ -24,10 +24,11 @@ export type State = { ...@@ -24,10 +24,11 @@ export type State = {
*/ */
annotationResultCount: number | null; annotationResultCount: number | null;
/** /** Count of remaining annotation imports. */
* Count of annotations waiting to be imported.
*/
importsPending: number; importsPending: number;
/** Total number of imports in active import tasks. */
importsTotal: number;
}; };
const initialState: State = { const initialState: State = {
...@@ -37,6 +38,7 @@ const initialState: State = { ...@@ -37,6 +38,7 @@ const initialState: State = {
hasFetchedAnnotations: false, hasFetchedAnnotations: false,
annotationResultCount: null, annotationResultCount: null,
importsPending: 0, importsPending: 0,
importsTotal: 0,
}; };
const reducers = { const reducers = {
...@@ -116,6 +118,7 @@ const reducers = { ...@@ -116,6 +118,7 @@ const reducers = {
BEGIN_IMPORT(state: State, action: { count: number }) { BEGIN_IMPORT(state: State, action: { count: number }) {
return { return {
importsPending: state.importsPending + action.count, importsPending: state.importsPending + action.count,
importsTotal: state.importsTotal + action.count,
}; };
}, },
...@@ -123,8 +126,12 @@ const reducers = { ...@@ -123,8 +126,12 @@ const reducers = {
if (!state.importsPending) { if (!state.importsPending) {
return state; return state;
} }
const importsPending = Math.max(state.importsPending - action.count, 0);
const importsTotal = importsPending > 0 ? state.importsTotal : 0;
return { return {
importsPending: Math.max(state.importsPending - action.count, 0), importsPending,
importsTotal,
}; };
}, },
}; };
...@@ -179,6 +186,10 @@ function importsPending(state: State) { ...@@ -179,6 +186,10 @@ function importsPending(state: State) {
return state.importsPending; return state.importsPending;
} }
function importsTotal(state: State) {
return state.importsTotal;
}
/** /**
* Return true when annotations are actively being fetched. * Return true when annotations are actively being fetched.
*/ */
...@@ -225,6 +236,7 @@ export const activityModule = createStoreModule(initialState, { ...@@ -225,6 +236,7 @@ export const activityModule = createStoreModule(initialState, {
selectors: { selectors: {
hasFetchedAnnotations, hasFetchedAnnotations,
importsPending, importsPending,
importsTotal,
isLoading, isLoading,
isFetchingAnnotations, isFetchingAnnotations,
isSavingAnnotation, isSavingAnnotation,
......
...@@ -225,28 +225,37 @@ describe('sidebar/store/modules/activity', () => { ...@@ -225,28 +225,37 @@ describe('sidebar/store/modules/activity', () => {
}); });
describe('#beginImport', () => { describe('#beginImport', () => {
it('increments count of pending imports', () => { it('updates count of pending and total imports', () => {
assert.equal(store.importsPending(), 0); assert.equal(store.importsPending(), 0);
store.beginImport(2); store.beginImport(2);
assert.equal(store.importsPending(), 2); assert.equal(store.importsPending(), 2);
assert.equal(store.importsTotal(), 2);
store.beginImport(2); store.beginImport(2);
assert.equal(store.importsPending(), 4); assert.equal(store.importsPending(), 4);
assert.equal(store.importsTotal(), 4);
}); });
}); });
describe('#completeImport', () => { describe('#completeImport', () => {
it('decrements count of pending imports', () => { it('updates count of pending and total imports', () => {
store.beginImport(5); store.beginImport(5);
store.completeImport(2); store.completeImport(2);
assert.equal(store.importsPending(), 3); assert.equal(store.importsPending(), 3);
assert.equal(store.importsTotal(), 5);
store.completeImport(1); store.completeImport(1);
assert.equal(store.importsPending(), 2); assert.equal(store.importsPending(), 2);
assert.equal(store.importsTotal(), 5);
store.completeImport(2); store.completeImport(2);
assert.equal(store.importsPending(), 0); assert.equal(store.importsPending(), 0);
// Once all the imports have completed, the count of total imports is
// reset.
assert.equal(store.importsTotal(), 0);
// Value can't go below 0. We could choose to throw an error here instead. // Value can't go below 0. We could choose to throw an error here instead.
store.completeImport(1); store.completeImport(1);
assert.equal(store.importsPending(), 0); assert.equal(store.importsPending(), 0);
assert.equal(store.importsTotal(), 0);
}); });
}); });
}); });
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