From 15f69849941cf68346080940ade64876b1e51757 Mon Sep 17 00:00:00 2001 From: Jeremy Press Date: Mon, 27 Aug 2018 15:58:08 -0700 Subject: [PATCH] Chore: Adding find bar metrics (#835) --- src/lib/events.js | 6 ++++ src/lib/viewers/doc/DocBaseViewer.js | 15 ++++++++++ src/lib/viewers/doc/DocFindBar.js | 15 ++++++++++ .../doc/__tests__/DocBaseViewer-test.js | 10 +++++-- .../viewers/doc/__tests__/DocFindBar-test.js | 30 ++++++++++++++++++- 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/lib/events.js b/src/lib/events.js index fd4f4aa3a..3632faf64 100644 --- a/src/lib/events.js +++ b/src/lib/events.js @@ -68,3 +68,9 @@ export const DOWNLOAD_REACHABILITY_METRICS = { NOTIFICATION_SHOWN: 'dl_reachability_notification_shown', DOWNLOAD_BLOCKED: 'dl_reachability_host_blocked' }; +// Events fired when using find in preview +export const USER_DOCUMENT_FIND_EVENTS = { + NEXT: 'user_document_find_next', // The user navigates to the next find entry + OPEN: 'user_document_find_open', // The user opens the find bar + PREVIOUS: 'user_document_find_previous' // The user navigates to the previous find entry +}; diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index b9169e941..e550e951d 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -74,6 +74,7 @@ class DocBaseViewer extends BaseViewer { this.pinchToZoomStartHandler = this.pinchToZoomStartHandler.bind(this); this.pinchToZoomChangeHandler = this.pinchToZoomChangeHandler.bind(this); this.pinchToZoomEndHandler = this.pinchToZoomEndHandler.bind(this); + this.emitMetric = this.emitMetric.bind(this); } /** @@ -132,6 +133,7 @@ class DocBaseViewer extends BaseViewer { // Clean up the find bar if (this.findBar) { this.findBar.destroy(); + this.findBar.removeListener(VIEWER_EVENT.metric, this.emitMetric); } // Clean up PDF network requests @@ -317,6 +319,7 @@ class DocBaseViewer extends BaseViewer { return; } this.findBar = new DocFindBar(this.findBarEl, this.findController, canDownload); + this.findBar.addListener(VIEWER_EVENT.metric, this.emitMetric); } /** @@ -527,6 +530,18 @@ class DocBaseViewer extends BaseViewer { return true; } + /** + * Emits a viewer metric. Useful for unpacking a message that comes from another class. + * + * @protected + * @emits metric + * @param {Object} event - Event object + * @return {void} + */ + emitMetric(event) { + super.emitMetric(event.name, event.data); + } + //-------------------------------------------------------------------------- // Protected //-------------------------------------------------------------------------- diff --git a/src/lib/viewers/doc/DocFindBar.js b/src/lib/viewers/doc/DocFindBar.js index dd3b6a168..48c2a665a 100644 --- a/src/lib/viewers/doc/DocFindBar.js +++ b/src/lib/viewers/doc/DocFindBar.js @@ -1,5 +1,6 @@ import EventEmitter from 'events'; import { decodeKeydown } from '../../util'; +import { USER_DOCUMENT_FIND_EVENTS, VIEWER_EVENT } from '../../events'; import { CLASS_HIDDEN } from '../../constants'; import { ICON_FIND_DROP_DOWN, ICON_FIND_DROP_UP, ICON_CLOSE, ICON_SEARCH } from '../../icons/icons'; @@ -328,6 +329,11 @@ class DocFindBar extends EventEmitter { this.currentMatch = 1; } } + + // Emit a metric that the user navigated forward in the find bar + this.emit(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.NEXT + }); } } @@ -351,6 +357,11 @@ class DocFindBar extends EventEmitter { this.currentMatch = this.findController.matchCount; } } + + // Emit a metric that the user navigated back in the find bar + this.emit(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.PREVIOUS + }); } } @@ -370,6 +381,10 @@ class DocFindBar extends EventEmitter { if (!this.opened) { this.opened = true; this.bar.classList.remove(CLASS_HIDDEN); + // Emit a metric that the user opened the find bar + this.emit(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.OPEN + }); } this.findFieldEl.select(); this.findFieldEl.focus(); diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 512a4b06e..de9cdfc71 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -123,11 +123,13 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { it('should destroy the find bar', () => { docBase.findBar = { - destroy: sandbox.stub() + destroy: sandbox.stub(), + removeListener: sandbox.stub() }; docBase.destroy(); expect(docBase.findBar.destroy).to.be.called; + expect(docBase.findBar.removeListener).to.be.called; }); it('should clean up the PDF network requests', () => { @@ -481,7 +483,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { setFindFieldElValue: sandbox.stub(), findFieldHandler: sandbox.stub(), open: sandbox.stub(), - destroy: sandbox.stub() + destroy: sandbox.stub(), + removeListener: sandbox.stub() }; sandbox.stub(docBase, 'setPage'); @@ -791,7 +794,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { const onKeydownStub = sandbox.stub().withArgs(mockEvent); docBase.findBar = { onKeydown: onKeydownStub, - destroy: sandbox.stub() + destroy: sandbox.stub(), + removeListener: sandbox.stub() }; docBase.onKeydown(keys, mockEvent); expect(onKeydownStub).to.have.been.calledOnce; diff --git a/src/lib/viewers/doc/__tests__/DocFindBar-test.js b/src/lib/viewers/doc/__tests__/DocFindBar-test.js index 11d50b9a9..7988b1265 100644 --- a/src/lib/viewers/doc/__tests__/DocFindBar-test.js +++ b/src/lib/viewers/doc/__tests__/DocFindBar-test.js @@ -2,6 +2,7 @@ import DocFindBar from '../DocFindBar'; import { CLASS_HIDDEN } from '../../../constants'; import * as util from '../../../util'; +import { VIEWER_EVENT, USER_DOCUMENT_FIND_EVENTS } from '../../../events'; const CLASS_FIND_MATCH_NOT_FOUND = 'bp-find-match-not-found'; @@ -233,7 +234,7 @@ describe('lib/viewers/doc/DocFindBar', () => { it('should set the findFieldEl value', () => { docFindBar.findFieldEl = { removeEventListener: sandbox.stub() - } + }; docFindBar.setFindFieldElValue('test'); @@ -479,6 +480,18 @@ describe('lib/viewers/doc/DocFindBar', () => { docFindBar.findNextHandler(true); expect(docFindBar.currentMatch).to.equal(1); }); + + it('should emit the find next event', () => { + sandbox.stub(docFindBar, 'emit'); + docFindBar.findFieldEl.value = 'test'; + docFindBar.findController.matchCount = 1; + docFindBar.currentMatch = 2; + + docFindBar.findNextHandler(true); + expect(docFindBar.emit).to.be.calledWith(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.NEXT + }); + }); }); describe('findPreviousHandler()', () => { @@ -517,6 +530,17 @@ describe('lib/viewers/doc/DocFindBar', () => { docFindBar.findPreviousHandler(true); expect(docFindBar.currentMatch).to.equal(5); }); + + it('should emit a find previous metric', () => { + sandbox.stub(docFindBar, 'emit'); + docFindBar.findFieldEl.value = 'test'; + docFindBar.currentMatch = 0; + + docFindBar.findPreviousHandler(true); + expect(docFindBar.emit).to.be.calledWith(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.PREVIOUS + }); + }); }); describe('open()', () => { beforeEach(() => { @@ -542,11 +566,15 @@ describe('lib/viewers/doc/DocFindBar', () => { }); it('should open the find bar if it is not open', () => { + sandbox.stub(docFindBar, 'emit'); docFindBar.opened = false; docFindBar.open(); expect(docFindBar.opened).to.equal(true); expect(stubs.remove).to.be.called; + expect(docFindBar.emit).to.be.calledWith(VIEWER_EVENT.metric, { + name: USER_DOCUMENT_FIND_EVENTS.OPEN + }); }); it('should not open the find bar if it is already open', () => {