fix(a11y): resolve comments

box · Aug 3, 2023 · e8cb1ab · e8cb1ab
1 parent 93a3c22
commit e8cb1ab
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 39 deletions.
diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js
@@ -177,11 +177,6 @@ class DocBaseViewer extends BaseViewer {
 
         this.viewerEl = this.docEl.appendChild(document.createElement('div'));
         this.viewerEl.classList.add('pdfViewer');
-        // Text layer should be rendered for a11y reasons thats why we will block user from selecting content when no download permissions was granted
-        const isViewOnly = !checkPermission(this.options.file, PERMISSION_DOWNLOAD);
-        if (isViewOnly) {
-            this.viewerEl.classList.add('viewOnly');
-        }
 
         this.loadTimeout = LOAD_TIMEOUT_MS;
 
@@ -801,10 +796,16 @@ class DocBaseViewer extends BaseViewer {
         const { AnnotationMode: PDFAnnotationMode = {} } = this.pdfjsLib;
         const assetUrlCreator = createAssetUrlCreator(location);
         const hasDownload = checkPermission(file, PERMISSION_DOWNLOAD);
+        const hasTextLayer = !this.getViewerOption('disableTextLayer');
         const enabledTextLayerMode = hasDownload
             ? PDFJS_TEXT_LAYER_MODE.ENABLE
             : PDFJS_TEXT_LAYER_MODE.ENABLE_PERMISSIONS; // This mode will prevent default behavior for copy events in the TextLayerBuilder
 
+        // Text layer should be rendered for a11y reasons thats why we will block user from selecting content when no download permissions was granted
+        if (!hasDownload) {
+            this.viewerEl.classList.add('pdfViewer--viewOnly');
+        }
+
         return new PdfViewerClass({
             annotationMode: PDFAnnotationMode.ENABLE, // Show annotations, but not forms
             container: this.docEl,
@@ -814,9 +815,7 @@ class DocBaseViewer extends BaseViewer {
             linkService: this.pdfLinkService,
             maxCanvasPixels: this.isMobile ? MOBILE_MAX_CANVAS_SIZE : -1,
             renderInteractiveForms: false, // Enabling prevents unverified signatures from being displayed
-            textLayerMode: this.getViewerOption('disableTextLayer')
-                ? PDFJS_TEXT_LAYER_MODE.DISABLE
-                : enabledTextLayerMode,
+            textLayerMode: hasTextLayer ? enabledTextLayerMode : PDFJS_TEXT_LAYER_MODE.DISABLE,
         });
     }
 

diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
@@ -276,35 +276,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             docBase.setup();
             expect(docBase.pageTracker).toBeInstanceOf(PageTracker);
         });
-        test('should add view only class if user does not have download permissions', () => {
-            docBase = new DocBaseViewer({
-                file: {
-                    id: '0',
-                },
-            });
-            docBase.containerEl = containerEl;
-
-            jest.spyOn(file, 'checkPermission').mockReturnValue(false);
-            docBase.setup();
-
-            expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD);
-            expect(docBase.viewerEl).toHaveClass('viewOnly');
-        });
-
-        test('should not add view only class if user has download permissions', () => {
-            docBase = new DocBaseViewer({
-                file: {
-                    id: '0',
-                },
-            });
-            docBase.containerEl = containerEl;
-
-            jest.spyOn(file, 'checkPermission').mockReturnValue(true);
-            docBase.setup();
-
-            expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD);
-            expect(docBase.viewerEl).not.toHaveClass('viewOnly');
-        });
     });
 
     describe('Non setup methods', () => {
@@ -1180,6 +1151,26 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
                 });
             });
 
+            test('should add view only class if user does not have download permissions', () => {
+                docBase.containerEl = containerEl;
+                stubs.checkPermission.mockReturnValueOnce(false);
+
+                return docBase.initViewer('').then(() => {
+                    expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD);
+                    expect(docBase.viewerEl).toHaveClass('pdfViewer--viewOnly');
+                });
+            });
+
+            test('should not add view only class if user has download permissions', () => {
+                docBase.containerEl = containerEl;
+                stubs.checkPermission.mockReturnValueOnce(true);
+
+                return docBase.initViewer('').then(() => {
+                    expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD);
+                    expect(docBase.viewerEl).not.toHaveClass('pdfViewer--viewOnly');
+                });
+            });
+
             test('should use proper text layer mode if user has download permissions and disableTextLayer viewer option is not set', () => {
                 stubs.getViewerOption.mockReturnValue(false);
                 stubs.checkPermission.mockReturnValueOnce(true);

diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss
@@ -305,10 +305,10 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border
     }
 
     .pdfViewer {
-        &.viewOnly {
+        &--viewOnly {
             .textLayer {
                 span {
-                    cursor: not-allowed;
+                    cursor: unset;
                     user-select: none;
                 }
             }