From 00b1f6542ab9abc911c23fc6192500847a399366 Mon Sep 17 00:00:00 2001 From: Priyajeet Hora Date: Tue, 21 Aug 2018 15:58:35 -0700 Subject: [PATCH] Fix: Preview should not auto focus itself unless told to do so --- README.md | 1 + src/lib/Preview.js | 5 +++- src/lib/__tests__/Preview-test.js | 23 +++++++++++++++++++ src/lib/viewers/media/DashViewer.js | 5 +++- src/lib/viewers/media/MediaBaseViewer.js | 5 +++- .../media/__tests__/DashViewer-test.js | 1 + .../media/__tests__/MediaBaseViewer-test.js | 1 + 7 files changed, 38 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8dae9e3e0..b8fae1585 100644 --- a/README.md +++ b/README.md @@ -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) | diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 98c8fad15..4f906c026 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -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; @@ -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(); } diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js index b5f865408..ecb3ca561 100644 --- a/src/lib/__tests__/Preview-test.js +++ b/src/lib/__tests__/Preview-test.js @@ -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); @@ -1905,6 +1915,7 @@ describe('lib/Preview', () => { }); it('should focus the viewer container', () => { + preview.options.autoFocus = true; preview.viewer.containerEl = { focus: () => {} }; @@ -1912,6 +1923,18 @@ describe('lib/Preview', () => { 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; diff --git a/src/lib/viewers/media/DashViewer.js b/src/lib/viewers/media/DashViewer.js index a32a256bd..89f7a9f34 100644 --- a/src/lib/viewers/media/DashViewer.js +++ b/src/lib/viewers/media/DashViewer.js @@ -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(); + } } /** diff --git a/src/lib/viewers/media/MediaBaseViewer.js b/src/lib/viewers/media/MediaBaseViewer.js index c489ccbed..066a3eebb 100644 --- a/src/lib/viewers/media/MediaBaseViewer.js +++ b/src/lib/viewers/media/MediaBaseViewer.js @@ -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(); + } } /** diff --git a/src/lib/viewers/media/__tests__/DashViewer-test.js b/src/lib/viewers/media/__tests__/DashViewer-test.js index 81cbdd012..2969bf3c3 100644 --- a/src/lib/viewers/media/__tests__/DashViewer-test.js +++ b/src/lib/viewers/media/__tests__/DashViewer-test.js @@ -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; diff --git a/src/lib/viewers/media/__tests__/MediaBaseViewer-test.js b/src/lib/viewers/media/__tests__/MediaBaseViewer-test.js index 71baec65d..d58812356 100644 --- a/src/lib/viewers/media/__tests__/MediaBaseViewer-test.js +++ b/src/lib/viewers/media/__tests__/MediaBaseViewer-test.js @@ -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;