Fix: Temporarily remove focus outline from scrollable content

box · Apr 15, 2019 · a8e574b · a8e574b
1 parent bd51d09
commit a8e574b
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 6 deletions.
diff --git a/src/lib/_common.scss b/src/lib/_common.scss
@@ -161,6 +161,10 @@ $header-height: 48px;
         background-color: inherit; // Safari needs some reminder of what to do for flex items in a flex container when in fullscreen
         width: 100%;
     }
+
+    .bp-is-scrollable:focus {
+        outline: none; // temporarily remove focus ring until we know how we want to indicate focus
+    }
 }
 
 .accessibility-hidden {

diff --git a/src/lib/viewers/image/MultiImageViewer.js b/src/lib/viewers/image/MultiImageViewer.js
@@ -2,7 +2,7 @@ import ImageBaseViewer from './ImageBaseViewer';
 import PageControls from '../../PageControls';
 import './MultiImage.scss';
 import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../icons/icons';
-import { CLASS_INVISIBLE, CLASS_MULTI_IMAGE_PAGE } from '../../constants';
+import { CLASS_INVISIBLE, CLASS_MULTI_IMAGE_PAGE, CLASS_IS_SCROLLABLE } from '../../constants';
 import { pageNumberFromScroll, fetchRepresentationAsBlob } from '../../util';
 
 const PADDING_BUFFER = 100;
@@ -37,7 +37,7 @@ class MultiImageViewer extends ImageBaseViewer {
         super.setup();
 
         this.wrapperEl = this.createViewer(document.createElement('div'));
-        this.wrapperEl.classList.add(CSS_CLASS_IMAGE_WRAPPER);
+        this.wrapperEl.className = `${CSS_CLASS_IMAGE_WRAPPER} ${CLASS_IS_SCROLLABLE}`;
         this.wrapperEl.tabIndex = '0';
 
         this.imageEl = this.wrapperEl.appendChild(document.createElement('div'));

diff --git a/src/lib/viewers/text/PlainTextViewer.js b/src/lib/viewers/text/PlainTextViewer.js
@@ -3,7 +3,7 @@ import api from '../../api';
 import TextBaseViewer from './TextBaseViewer';
 import Browser from '../../Browser';
 import Popup from '../../Popup';
-import { CLASS_HIDDEN, TEXT_STATIC_ASSETS_VERSION } from '../../constants';
+import { CLASS_HIDDEN, TEXT_STATIC_ASSETS_VERSION, CLASS_IS_SCROLLABLE } from '../../constants';
 import { ICON_PRINT_CHECKMARK } from '../../icons/icons';
 import { HIGHLIGHTTABLE_EXTENSIONS } from '../../extensions';
 import { openContentInsideIframe, createAssetUrlCreator, createStylesheet } from '../../util';
@@ -113,8 +113,7 @@ class PlainTextViewer extends TextBaseViewer {
         super.setup();
 
         this.textEl = this.createViewer(document.createElement('pre'));
-        this.textEl.className = 'bp-text bp-text-plain hljs';
-        this.textEl.classList.add(CLASS_HIDDEN);
+        this.textEl.className = `bp-text bp-text-plain hljs ${CLASS_IS_SCROLLABLE} ${CLASS_HIDDEN}`;
         this.textEl.tabIndex = '0';
 
         this.codeEl = this.textEl.appendChild(document.createElement('code'));

diff --git a/src/lib/viewers/text/__tests__/PlainTextViewer-test.js b/src/lib/viewers/text/__tests__/PlainTextViewer-test.js
@@ -72,7 +72,7 @@ describe('lib/viewers/text/PlainTextViewer', () => {
 
             text.setup();
 
-            expect(text.textEl.className).to.equal('bp-text bp-text-plain hljs bp-is-hidden');
+            expect(text.textEl.className).to.equal('bp-text bp-text-plain hljs bp-is-scrollable bp-is-hidden');
             expect(text.codeEl.parentNode === text.textEl).to.be.true;
             expect(text.truncated).to.be.false;
             expect(text.initPrint).to.be.called;