Skip to content

Commit

Permalink
fix(find): Prevent loss of focus on content when find bar is closed (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jstoffan authored and mergify[bot] committed Aug 15, 2019
1 parent 28fd209 commit 0a210dd
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 27 deletions.
29 changes: 19 additions & 10 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,22 @@ class DocBaseViewer extends BaseViewer {
constructor(options) {
super(options);
// Bind context for callbacks
this.emitMetric = this.emitMetric.bind(this);
this.handleAssetAndRepLoad = this.handleAssetAndRepLoad.bind(this);
this.print = this.print.bind(this);
this.setPage = this.setPage.bind(this);
this.zoomIn = this.zoomIn.bind(this);
this.zoomOut = this.zoomOut.bind(this);
this.pagerenderedHandler = this.pagerenderedHandler.bind(this);
this.handleFindBarClose = this.handleFindBarClose.bind(this);
this.onThumbnailSelectHandler = this.onThumbnailSelectHandler.bind(this);
this.pagechangeHandler = this.pagechangeHandler.bind(this);
this.pagerenderedHandler = this.pagerenderedHandler.bind(this);
this.pagesinitHandler = this.pagesinitHandler.bind(this);
this.throttledScrollHandler = this.getScrollHandler().bind(this);
this.pinchToZoomStartHandler = this.pinchToZoomStartHandler.bind(this);
this.pinchToZoomChangeHandler = this.pinchToZoomChangeHandler.bind(this);
this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this);
this.emitMetric = this.emitMetric.bind(this);
this.pinchToZoomStartHandler = this.pinchToZoomStartHandler.bind(this);
this.print = this.print.bind(this);
this.setPage = this.setPage.bind(this);
this.throttledScrollHandler = this.getScrollHandler().bind(this);
this.toggleThumbnails = this.toggleThumbnails.bind(this);
this.onThumbnailSelectHandler = this.onThumbnailSelectHandler.bind(this);
this.zoomIn = this.zoomIn.bind(this);
this.zoomOut = this.zoomOut.bind(this);
}

/**
Expand Down Expand Up @@ -346,6 +347,12 @@ class DocBaseViewer extends BaseViewer {
super.handleAssetAndRepLoad();
}

handleFindBarClose() {
if (this.docEl) {
this.docEl.focus(); // Prevent focus from transferring to the root document element
}
}

/**
* Initializes the Find Bar and Find Controller
*
Expand All @@ -365,11 +372,13 @@ class DocBaseViewer extends BaseViewer {
// the file. Users without download permissions shouldn't be able to
// interact with the text layer
const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD);
if (this.getViewerOption('disableFindBar')) {
if (!canDownload || this.getViewerOption('disableFindBar')) {
return;
}

this.findBar = new DocFindBar(this.findBarEl, this.findController, canDownload);
this.findBar.addListener(VIEWER_EVENT.metric, this.emitMetric);
this.findBar.addListener('close', this.handleFindBarClose);
}

/**
Expand Down
9 changes: 3 additions & 6 deletions src/lib/viewers/doc/DocFindBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@ class DocFindBar extends EventEmitter {
*
* @param {string|HTMLElement} findBar - Find bar selector or element
* @param {Object} findController - Document find controller to use
* @param {boolean} canDownload - Whether user can download document or not
* @return {DocFindBar} DocFindBar instance
*/
constructor(findBar, findController, canDownload) {
constructor(findBar, findController) {
super();

this.opened = false;
this.bar = findBar;
this.findController = findController;
this.currentMatch = 0;
this.canDownload = canDownload;

if (this.findController === null) {
throw new Error('DocFindBar cannot be used without a PDFFindController instance.');
Expand Down Expand Up @@ -249,9 +247,7 @@ class DocFindBar extends EventEmitter {
case 'meta+g':
case 'control+g':
case 'f3':
if (this.canDownload) {
this.open();
}
this.open();
event.preventDefault();
break;
default:
Expand Down Expand Up @@ -405,6 +401,7 @@ class DocFindBar extends EventEmitter {
if (!this.opened) {
return;
}
this.emit('close');
this.opened = false;
this.bar.classList.add(CLASS_HIDDEN);
this.findController.active = false;
Expand Down
9 changes: 8 additions & 1 deletion src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
stubs.api = new Api();
stubs.classListAdd = sandbox.stub(rootEl.classList, 'add');
stubs.classListRemove = sandbox.stub(rootEl.classList, 'remove');
stubs.checkPermission = sandbox.stub(file, 'checkPermission');
});

afterEach(() => {
Expand Down Expand Up @@ -675,7 +676,14 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
expect(docBase.findBar).to.be.undefined;
});

it('should not set find bar if the user does not have download permissions', () => {
stubs.checkPermission.withArgs(docBase.options.file, PERMISSION_DOWNLOAD).returns(false);
docBase.initFind();
expect(docBase.findBar).to.be.undefined;
});

it('should set findBar to a function if viewer option disableFindBar is not set', () => {
stubs.checkPermission.withArgs(docBase.options.file, PERMISSION_DOWNLOAD).returns(true);
docBase.initFind();
expect(docBase.findBar).to.be.instanceof(DocFindBar);
});
Expand Down Expand Up @@ -1405,7 +1413,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
return 'asset';
});
stubs.browser = sandbox.stub(Browser, 'getName').returns('Safari');
stubs.checkPermission = sandbox.stub(file, 'checkPermission');
stubs.getViewerOption = sandbox.stub(docBase, 'getViewerOption');
docBase.options = {
location: {
Expand Down
13 changes: 3 additions & 10 deletions src/lib/viewers/doc/__tests__/DocFindBar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ describe('lib/viewers/doc/DocFindBar', () => {
expect(docFindBar.bar).to.equal(findBarEl);
expect(docFindBar.findController).to.equal(findController);
expect(docFindBar.currentMatch).to.equal(0);
expect(docFindBar.canDownload).to.be.true;
});

it('should throw an error if there is no findController', () => {
Expand Down Expand Up @@ -287,15 +286,6 @@ describe('lib/viewers/doc/DocFindBar', () => {
stubs.close = sandbox.stub(docFindBar, 'close');
});

it('should prevent default but not open the find bar if downloads are disabled', () => {
docFindBar.canDownload = false;
stubs.decodeKeydown.returns('meta+f');

docFindBar.onKeydown(stubs.event);
expect(stubs.open).to.not.be.called;
expect(stubs.event.preventDefault).to.be.called;
});

it('should open and prevent default if meta+f is entered', () => {
stubs.decodeKeydown.returns('meta+f');

Expand Down Expand Up @@ -611,10 +601,13 @@ describe('lib/viewers/doc/DocFindBar', () => {
});

it('should hide the bar if it is open', () => {
sandbox.stub(docFindBar, 'emit');

docFindBar.findFieldEl.value = 'test';
docFindBar.opened = true;

docFindBar.close();
expect(docFindBar.emit).to.be.calledWith('close');
expect(docFindBar.opened).to.equal(false);
expect(stubs.add).to.be.calledWith(CLASS_HIDDEN);
expect(docFindBar.findController.active).to.equal(false);
Expand Down

0 comments on commit 0a210dd

Please sign in to comment.