Fix: Preview should not auto focus itself unless told to do so (#833)

box · Aug 22, 2018 · d5ce4ef · d5ce4ef
1 parent 063ebd5
commit d5ce4ef
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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,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;

diff --git 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
@@ -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
@@ -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
@@ -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;