Skip to content

Commit

Permalink
fix(a11y): Add keyboard support for highlight comment flow (#1422)
Browse files Browse the repository at this point in the history
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jstoffan and mergify[bot] authored Aug 31, 2021
1 parent 59bbd40 commit 5df2dd2
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 2 deletions.
3 changes: 3 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ box3d_settings_projection_label=Camera Projection
# Settings rotate model label
box3d_settings_rotate_label=Rotate Model

# Document Preview
document_label=Press Alt+Arrow key combinations for caret navigation and text selection within the document's contents

# Annotations
# Placeholder text for create textarea in annotation dialog
annotation_add_comment_placeholder=Add a comment here...
Expand Down
30 changes: 28 additions & 2 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@ import {
QUERY_PARAM_ENCODING,
STATUS_SUCCESS,
} from '../../constants';
import { appendQueryParams, createAssetUrlCreator, getMidpoint, getDistance, getClosestPageToPinch } from '../../util';
import {
appendQueryParams,
createAssetUrlCreator,
decodeKeydown,
getClosestPageToPinch,
getDistance,
getMidpoint,
} from '../../util';
import { checkPermission, getRepresentation } from '../../file';
import { ICON_PRINT_CHECKMARK } from '../../icons';
import { JS, PRELOAD_JS, CSS } from './docAssets';
Expand Down Expand Up @@ -106,6 +113,7 @@ class DocBaseViewer extends BaseViewer {
this.handleAnnotationControlsEscape = this.handleAnnotationControlsEscape.bind(this);
this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this);
this.handleAnnotationCreatorChangeEvent = this.handleAnnotationCreatorChangeEvent.bind(this);
this.handleDocElKeydown = this.handleDocElKeydown.bind(this);
this.handlePageSubmit = this.handlePageSubmit.bind(this);
this.onThumbnailSelectHandler = this.onThumbnailSelectHandler.bind(this);
this.pagechangingHandler = this.pagechangingHandler.bind(this);
Expand Down Expand Up @@ -140,6 +148,7 @@ class DocBaseViewer extends BaseViewer {
super.setup();

this.docEl = this.createViewer(document.createElement('div'));
this.docEl.setAttribute('aria-label', __('document_label'));
this.docEl.classList.add('bp-doc');
this.docEl.tabIndex = '0';

Expand Down Expand Up @@ -1114,7 +1123,7 @@ class DocBaseViewer extends BaseViewer {
* @return {void}
*/
bindDOMListeners() {
// Detects scroll so an event can be fired
this.docEl.addEventListener('keydown', this.handleDocElKeydown);
this.docEl.addEventListener('scroll', this.throttledScrollHandler);

if (this.hasTouch) {
Expand All @@ -1132,6 +1141,7 @@ class DocBaseViewer extends BaseViewer {
*/
unbindDOMListeners() {
if (this.docEl) {
this.docEl.removeEventListener('keydown', this.handleDocElKeydown);
this.docEl.removeEventListener('scroll', this.throttledScrollHandler);

if (this.hasTouch) {
Expand Down Expand Up @@ -1275,6 +1285,22 @@ class DocBaseViewer extends BaseViewer {
this.emit('pagefocus', pageNumber);
}

/**
* Handler for 'keydown' event on the bp-doc element. These conditions cannot be managed in onKeydown, as
* it listens for events on the top-level document element.
*
* @private
* @param {KeyboardEvent} event - Keydown event
* @return {void}
*/
handleDocElKeydown(event) {
const key = decodeKeydown(event);

if (event.altKey && key.includes('Arrow')) {
event.stopPropagation(); // Prevent collection/page navigation for caret navigation users
}
}

/** @inheritDoc */
handleFullscreenEnter() {
this.pdfViewer.currentScaleValue = 'page-fit';
Expand Down
29 changes: 29 additions & 0 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
docBase.hasTouch = false;
docBase.bindDOMListeners();

expect(stubs.addEventListener).toBeCalledWith('keydown', docBase.handleDocElKeydown);
expect(stubs.addEventListener).toBeCalledWith('scroll', docBase.throttledScrollHandler);
expect(stubs.addEventListener).not.toBeCalledWith('touchstart', docBase.pinchToZoomStartHandler);
expect(stubs.addEventListener).not.toBeCalledWith('touchmove', docBase.pinchToZoomChangeHandler);
Expand All @@ -1812,6 +1813,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {

test('should remove the docBase element listeners if the docBase element exists', () => {
docBase.unbindDOMListeners();

expect(stubs.removeEventListener).toBeCalledWith('keydown', docBase.handleDocElKeydown);
expect(stubs.removeEventListener).toBeCalledWith('scroll', docBase.throttledScrollHandler);
});

Expand Down Expand Up @@ -2009,6 +2012,32 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
});
});

describe.only('handleDocElKeydown()', () => {
test.each(['Enter', 'Escape', 'Space'])('should ignore %s key events', key => {
const event = { key, stopPropagation: jest.fn() };

docBase.handleDocElKeydown(event);

expect(event.stopPropagation).not.toBeCalled();
});

test.each(['Left', 'Right', 'Up', 'Down'])('should ignore %s key events without alt', key => {
const event = { altKey: false, key, stopPropagation: jest.fn() };

docBase.handleDocElKeydown(event);

expect(event.stopPropagation).not.toBeCalled();
});

test.each(['Left', 'Right', 'Up', 'Down'])('should stop propagation for %s key events with alt', key => {
const event = { altKey: true, key, stopPropagation: jest.fn() };

docBase.handleDocElKeydown(event);

expect(event.stopPropagation).toBeCalled();
});
});

describe('handleFullscreenEnter()', () => {
test('should update the scale value, and resize the page', () => {
docBase.pdfViewer = {
Expand Down
1 change: 1 addition & 0 deletions src/lib/viewers/doc/_docBase.scss
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border
}

.textLayer {
caret-color: black; // Required for caret navigation on transparent text
top: $pdfjs-page-padding;
bottom: auto;
opacity: 1;
Expand Down

0 comments on commit 5df2dd2

Please sign in to comment.