Skip to content

Commit

Permalink
chore(loading): Remove sub-header progress bar experience (#1348)
Browse files Browse the repository at this point in the history
  • Loading branch information
jstoffan authored Apr 5, 2021
1 parent 5d248d5 commit c40435e
Show file tree
Hide file tree
Showing 15 changed files with 6 additions and 418 deletions.
17 changes: 1 addition & 16 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,10 +870,9 @@ class Preview extends EventEmitter {
// Update navigation
this.ui.showNavigation(this.file.id, this.collection);

// Setup loading UI and progress bar
// Setup loading indicator
this.ui.showLoadingIcon(this.file.extension);
this.ui.showLoadingIndicator();
this.ui.startProgressBar();

// Start the preview duration timer when the user starts to perceive preview's load
const previewDurationTag = Timer.createTag(this.file.id, DURATION_METRIC);
Expand Down Expand Up @@ -929,9 +928,6 @@ class Preview extends EventEmitter {
// Whether the loading indicator should be shown
this.options.showLoading = options.showLoading !== false;

// Whether the progress indicator should be shown
this.options.showProgress = options.showProgress !== false;

// Whether annotations v4 buttons should be shown in toolbar
this.options.showAnnotationsControls = !!options.showAnnotationsControls;

Expand Down Expand Up @@ -1264,12 +1260,6 @@ class Preview extends EventEmitter {
case VIEWER_EVENT.load:
this.finishLoading(data.data);
break;
case VIEWER_EVENT.progressStart:
this.ui.startProgressBar();
break;
case VIEWER_EVENT.progressEnd:
this.ui.finishProgressBar();
break;
case VIEWER_EVENT.error:
// Do nothing since 'error' event was already caught, and will be emitted
// as a 'preview_error' event
Expand Down Expand Up @@ -1373,11 +1363,6 @@ class Preview extends EventEmitter {
}
}

// Finish the progress bar unless instructed not to
if (data.endProgress !== false) {
this.ui.finishProgressBar();
}

// Programmatically focus on the viewer after it loads
if (this.options.autoFocus && this.viewer && this.viewer.containerEl) {
this.viewer.containerEl.focus();
Expand Down
1 change: 0 additions & 1 deletion src/lib/Preview.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
@import 'loading';
@import 'navigation';
@import './Controls';
@import './ProgressBar';
@import './VirtualScroller';
37 changes: 0 additions & 37 deletions src/lib/PreviewUI.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import LoadingIcon from './LoadingIcon';
import Notification from './Notification';
import ProgressBar from './ProgressBar';
import shellTemplate from './shell.html';
import {
CLASS_BOX_PREVIEW_BASE_HEADER,
Expand Down Expand Up @@ -49,20 +48,13 @@ class PreviewUI {
/** @property {HTMLElement} - Preview container element which houses the sidebar and content */
previewContainer;

/** @property {ProgressBar} - Progress bar instance */
progressBar;

/**
* Destroy preview container content.
*
* @public
* @return {void}
*/
cleanup() {
if (this.progressBar) {
this.progressBar.destroy();
}

if (this.previewContainer) {
this.previewContainer.removeEventListener('mousemove', this.mousemoveHandler);
}
Expand Down Expand Up @@ -131,11 +123,6 @@ class PreviewUI {
this.destroyLoading();
}

// Setup progress bar
if (options.showProgress) {
this.progressBar = new ProgressBar(this.container);
}

// Attach keyboard events
document.addEventListener('keydown', this.keydownHandler);

Expand Down Expand Up @@ -274,30 +261,6 @@ class PreviewUI {
this.previewContainer.classList.add(CLASS_PREVIEW_LOADED);
}

/**
* Shows and starts a progress bar at the top of the preview.
*
* @public
* @return {void}
*/
startProgressBar() {
if (this.progressBar) {
this.progressBar.start();
}
}

/**
* Finishes and hides the top progress bar if present.
*
* @public
* @return {void}
*/
finishProgressBar() {
if (this.progressBar) {
this.progressBar.finish();
}
}

/**
* Setup notification
*
Expand Down
127 changes: 0 additions & 127 deletions src/lib/ProgressBar.js

This file was deleted.

22 changes: 0 additions & 22 deletions src/lib/ProgressBar.scss

This file was deleted.

38 changes: 0 additions & 38 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,6 @@ describe('lib/Preview', () => {
previewUIMock.expects('setup');
previewUIMock.expects('showLoadingIcon');
previewUIMock.expects('showLoadingIndicator');
previewUIMock.expects('startProgressBar');
previewUIMock.expects('showNavigation');
previewUIMock.expects('setupNotification');

Expand Down Expand Up @@ -1320,15 +1319,6 @@ describe('lib/Preview', () => {
expect(preview.options.showLoading).toBe(false);
});

test('should set whether to show progress indicators', () => {
preview.parseOptions(preview.previewOptions);
expect(preview.options.showProgress).toBe(true);

preview.previewOptions.showProgress = false;
preview.parseOptions(preview.previewOptions);
expect(preview.options.showProgress).toBe(false);
});

test('should set whether to skip load from the server and any server updates', () => {
preview.parseOptions(preview.previewOptions);
expect(preview.options.skipServerUpdate).toBe(false);
Expand Down Expand Up @@ -1784,11 +1774,6 @@ describe('lib/Preview', () => {
});

describe('handleViewerEvents()', () => {
beforeEach(() => {
jest.spyOn(preview.ui, 'startProgressBar').mockImplementation();
jest.spyOn(preview.ui, 'finishProgressBar').mockImplementation();
});

test('should call download on download event', () => {
jest.spyOn(preview, 'download').mockImplementation();
preview.handleViewerEvents({ event: VIEWER_EVENT.download });
Expand All @@ -1807,16 +1792,6 @@ describe('lib/Preview', () => {
expect(preview.finishLoading).toBeCalled();
});

test('should start progress bar on progressstart event', () => {
preview.handleViewerEvents({ event: VIEWER_EVENT.progressStart });
expect(preview.ui.startProgressBar).toBeCalled();
});

test('should finish progress bar on progressend event', () => {
preview.handleViewerEvents({ event: VIEWER_EVENT.progressEnd });
expect(preview.ui.finishProgressBar).toBeCalled();
});

test('should emit viewerevent when event does not match', () => {
jest.spyOn(preview, 'emit');
const data = {
Expand Down Expand Up @@ -1867,7 +1842,6 @@ describe('lib/Preview', () => {
stubs.emit = jest.spyOn(preview, 'emit');
stubs.logPreviewEvent = jest.spyOn(preview, 'logPreviewEvent');
stubs.prefetchNextFiles = jest.spyOn(preview, 'prefetchNextFiles');
stubs.finishProgressBar = jest.spyOn(preview.ui, 'finishProgressBar').mockImplementation();
stubs.setupNotification = jest.spyOn(preview.ui, 'setupNotification').mockImplementation();

stubs.perf = {
Expand Down Expand Up @@ -1970,18 +1944,6 @@ describe('lib/Preview', () => {
expect(callPhantomSpy).toBeCalled();
});

test('should postload if skipPostload is not true', () => {
preview.finishLoading();
expect(stubs.finishProgressBar).toBeCalled();
});

test('should skip postload if skipPostload is true', () => {
preview.finishLoading({
endProgress: false,
});
expect(stubs.finishProgressBar).not.toBeCalled();
});

test('should focus the viewer container', () => {
preview.options.autoFocus = true;
preview.viewer.containerEl = {
Expand Down
Loading

0 comments on commit c40435e

Please sign in to comment.