Skip to content

Commit

Permalink
Fix: Preview should not auto focus itself unless told to do so (#833)
Browse files Browse the repository at this point in the history
  • Loading branch information
priyajeet authored Aug 22, 2018
1 parent 063ebd5 commit d5ce4ef
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 3 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ preview.show(fileId, accessToken, {
| logoUrl | | URL of logo to show in header |
| showAnnotations | false | Whether annotations and annotation controls are shown. This option will be overridden by viewer-specific annotation options if they are set. See [Box Annotations](https://github.com/box/box-annotations) for more details |
| showDownload | false | Whether download button is shown |
| autoFocus | true | Whether preview should focus itself when a file loads |
| useHotkeys | true | Whether hotkeys (keyboard shortcuts) are enabled |
| fixDependencies | false | Temporarily patches AMD to properly load Preview's dependencies. You may need to enable this if your project uses RequireJS |
| disableEventLog | false | Disables client-side `preview` event log. Previewing with this option enabled will not increment access stats (content access is still logged server-side) |
Expand Down
5 changes: 4 additions & 1 deletion src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,9 @@ class Preview extends EventEmitter {
// Custom logo URL
this.options.logoUrl = options.logoUrl || '';

// Allow autofocussing
this.options.autoFocus = options.autoFocus !== false;

// Whether download button should be shown
this.options.showDownload = !!options.showDownload;

Expand Down Expand Up @@ -1307,7 +1310,7 @@ class Preview extends EventEmitter {
}

// Programmatically focus on the viewer after it loads
if (this.viewer && this.viewer.containerEl) {
if (this.options.autoFocus && this.viewer && this.viewer.containerEl) {
this.viewer.containerEl.focus();
}

Expand Down
23 changes: 23 additions & 0 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,16 @@ describe('lib/Preview', () => {
expect(preview.options.fixDependencies).to.be.true;
});

it('should allow auto focussing by default', () => {
preview.parseOptions(preview.previewOptions);
expect(preview.options.autoFocus).to.be.true;
});

it('should not allow auto focussing when told not to', () => {
preview.parseOptions({ ...preview.previewOptions, autoFocus: false });
expect(preview.options.autoFocus).to.be.false;
});

it('should add user created loaders before standard loaders', () => {
const expectedLoaders = stubs.loaders.concat(loaders);

Expand Down Expand Up @@ -1905,13 +1915,26 @@ describe('lib/Preview', () => {
});

it('should focus the viewer container', () => {
preview.options.autoFocus = true;
preview.viewer.containerEl = {
focus: () => {}
};
sandbox.mock(preview.viewer.containerEl).expects('focus');
preview.finishLoading();
});

it('should not focus the viewer container with autoFocus is false', () => {
preview.options.autoFocus = false;
preview.viewer.containerEl = {
focus: () => {}
};
sandbox
.mock(preview.viewer.containerEl)
.expects('focus')
.never();
preview.finishLoading();
});

it('should hide the loading indicator', () => {
preview.finishLoading();
expect(stubs.hideLoadingIndicator).to.be.called;
Expand Down
5 changes: 4 additions & 1 deletion src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,10 @@ class DashViewer extends VideoBaseViewer {
// Make media element visible after resize
this.showMedia();
this.mediaControls.show();
this.mediaContainerEl.focus();

if (this.options.autoFocus) {
this.mediaContainerEl.focus();
}
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/lib/viewers/media/MediaBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,10 @@ class MediaBaseViewer extends BaseViewer {

// Make media element visible after resize
this.showMedia();
this.mediaContainerEl.focus();

if (this.options.autoFocus) {
this.mediaContainerEl.focus();
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash, 'loadSubtitles');
sandbox.stub(dash, 'loadAlternateAudio');

dash.options.autoFocus = true;
dash.loadeddataHandler();
expect(dash.autoplay).to.be.called;
expect(dash.loadUI).to.be.called;
Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
sandbox.stub(media, 'resize');
sandbox.stub(media, 'showMedia');

media.options.autoFocus = true;
media.loadeddataHandler();

expect(media.handleVolume).to.be.called;
Expand Down

0 comments on commit d5ce4ef

Please sign in to comment.