Skip to content

Commit

Permalink
feat(discoverability): Add instrumentation for annotation creation (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan committed Oct 27, 2020
1 parent 7f11114 commit 480a90d
Show file tree
Hide file tree
Showing 8 changed files with 247 additions and 37 deletions.
60 changes: 39 additions & 21 deletions src/lib/AnnotationControlsFSM.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum AnnotationInput {
UPDATE = 'update',
}

type Subscription = (state: AnnotationState) => void;

export const modeStateMap = {
[AnnotationMode.HIGHLIGHT]: AnnotationState.HIGHLIGHT,
[AnnotationMode.NONE]: AnnotationState.NONE,
Expand All @@ -41,38 +43,54 @@ export const stateModeMap = {
export default class AnnotationControlsFSM {
private currentState: AnnotationState;

private subscriptions: Subscription[] = [];

constructor(initialState = AnnotationState.NONE) {
this.currentState = initialState;
}

public getState = (): AnnotationState => this.currentState;

public subscribe = (callback: Subscription): void => {
this.subscriptions.push(callback);
};

public transition = (input: AnnotationInput, mode: AnnotationMode = AnnotationMode.NONE): AnnotationMode => {
let newState = this.currentState;

if (input === AnnotationInput.CLICK) {
this.currentState = mode === stateModeMap[this.currentState] ? AnnotationState.NONE : modeStateMap[mode];
return stateModeMap[this.currentState];
newState = mode === stateModeMap[this.currentState] ? AnnotationState.NONE : modeStateMap[mode];
} else if (input === AnnotationInput.RESET) {
newState = AnnotationState.NONE;
} else {
switch (this.currentState) {
case AnnotationState.NONE:
if (input === AnnotationInput.CREATE || input === AnnotationInput.STARTED) {
newState = modeTempStateMap[mode] || AnnotationState.NONE;
}
break;
case AnnotationState.HIGHLIGHT_TEMP:
case AnnotationState.REGION_TEMP:
if (input === AnnotationInput.CANCEL || input === AnnotationInput.SUCCESS) {
newState = AnnotationState.NONE;
}
break;
default:
}
}

if (input === AnnotationInput.RESET) {
this.currentState = AnnotationState.NONE;
return stateModeMap[this.currentState];
}
this.setState(newState);
return stateModeMap[this.currentState];
};

switch (this.currentState) {
case AnnotationState.NONE:
if (input === AnnotationInput.CREATE || input === AnnotationInput.STARTED) {
this.currentState = modeTempStateMap[mode] || AnnotationState.NONE;
}
break;
case AnnotationState.HIGHLIGHT_TEMP:
case AnnotationState.REGION_TEMP:
if (input === AnnotationInput.CANCEL || input === AnnotationInput.SUCCESS) {
this.currentState = AnnotationState.NONE;
}
break;
default:
}
public unsubscribe = (callback: Subscription): void => {
this.subscriptions = this.subscriptions.filter(subscription => subscription !== callback);
};

return stateModeMap[this.currentState];
private notify = (): void => this.subscriptions.forEach(callback => callback(this.currentState));

private setState = (state: AnnotationState): void => {
this.currentState = state;
this.notify();
};
}
25 changes: 25 additions & 0 deletions src/lib/__tests__/AnnotationControlsFSM-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,29 @@ describe('lib/AnnotationControlsFSM', () => {
});
});
});

describe('subscriptions', () => {
const fsm = new AnnotationControlsFSM();
let callback;

beforeEach(() => {
callback = jest.fn();
fsm.subscribe(callback);
});

test('should call callback on transition with new state', () => {
fsm.transition(AnnotationInput.CLICK, AnnotationMode.REGION);
expect(callback).toHaveBeenCalledWith(fsm.getState());
});

test('should handle unsubscribe', () => {
const callbackTwo = jest.fn();
fsm.subscribe(callbackTwo);
fsm.unsubscribe(callback);

fsm.transition(AnnotationInput.CLICK, AnnotationMode.REGION);
expect(callback).not.toHaveBeenCalled();
expect(callbackTwo).toHaveBeenCalledWith(fsm.getState());
});
});
});
2 changes: 2 additions & 0 deletions src/lib/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export const CLASS_IS_BUFFERING = 'bp-is-buffering';
export const CLASS_DARK = 'bp-dark';
export const CLASS_CRAWLER = 'bp-crawler';

export const DISCOVERABILITY_ATTRIBUTE = 'data-resin-discoverability';

export const SELECTOR_BOX_PREVIEW_CONTAINER = `.${CLASS_BOX_PREVIEW_CONTAINER}`;
export const SELECTOR_BOX_PREVIEW = `.${CLASS_BOX_PREVIEW}`;
export const SELECTOR_BOX_PREVIEW_CONTENT = `.${CLASS_BOX_PREVIEW_CONTENT}`;
Expand Down
16 changes: 8 additions & 8 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
/* eslint-disable no-unused-expressions */
import EventEmitter from 'events';
import * as constants from '../../constants';
import * as icons from '../../icons/icons';
import * as util from '../../util';
import Api from '../../api';
import BaseViewer from '../BaseViewer';
import Browser from '../../Browser';
import RepStatus from '../../RepStatus';
import PreviewError from '../../PreviewError';
import fullscreen from '../../Fullscreen';
import intl from '../../i18n';
import * as util from '../../util';
import * as icons from '../../icons/icons';
import * as constants from '../../constants';
import PreviewError from '../../PreviewError';
import RepStatus from '../../RepStatus';
import Timer from '../../Timer';
import { AnnotationMode } from '../../AnnotationControls';
import { ERROR_CODE, LOAD_METRIC, VIEWER_EVENT } from '../../events';
import { EXCLUDED_EXTENSIONS } from '../../extensions';
import { VIEWER_EVENT, LOAD_METRIC, ERROR_CODE } from '../../events';
import Timer from '../../Timer';
import Api from '../../api';

let base;
let containerEl;
Expand Down
28 changes: 27 additions & 1 deletion src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Popup from '../../Popup';
import RepStatus from '../../RepStatus';
import PreviewError from '../../PreviewError';
import ThumbnailsSidebar from '../../ThumbnailsSidebar';
import { AnnotationInput } from '../../AnnotationControlsFSM';
import { AnnotationInput, AnnotationState } from '../../AnnotationControlsFSM';
import {
ANNOTATOR_EVENT,
CLASS_BOX_PREVIEW_THUMBNAILS_CLOSE_ACTIVE,
Expand All @@ -21,6 +21,7 @@ import {
CLASS_CRAWLER,
CLASS_HIDDEN,
CLASS_IS_SCROLLABLE,
DISCOVERABILITY_ATTRIBUTE,
DOC_STATIC_ASSETS_VERSION,
ENCODING_TYPES,
PERMISSION_DOWNLOAD,
Expand Down Expand Up @@ -48,6 +49,12 @@ import {
} from '../../events';
import Timer from '../../Timer';

export const DISCOVERABILITY_STATES = [
AnnotationState.HIGHLIGHT_TEMP,
AnnotationState.NONE,
AnnotationState.REGION_TEMP,
];

const CURRENT_PAGE_MAP_KEY = 'doc-current-page-map';
const DEFAULT_SCALE_DELTA = 0.1;
const IS_SAFARI_CLASS = 'is-safari';
Expand Down Expand Up @@ -113,8 +120,11 @@ class DocBaseViewer extends BaseViewer {
this.setPage = this.setPage.bind(this);
this.throttledScrollHandler = this.getScrollHandler().bind(this);
this.toggleThumbnails = this.toggleThumbnails.bind(this);
this.updateDiscoverabilityResinTag = this.updateDiscoverabilityResinTag.bind(this);
this.zoomIn = this.zoomIn.bind(this);
this.zoomOut = this.zoomOut.bind(this);

this.annotationControlsFSM.subscribe(this.updateDiscoverabilityResinTag);
}

/**
Expand Down Expand Up @@ -158,6 +168,8 @@ class DocBaseViewer extends BaseViewer {
this.thumbnailsSidebarEl.tabIndex = 0;
this.rootEl.insertBefore(this.thumbnailsSidebarEl, this.containerEl);
}

this.updateDiscoverabilityResinTag();
}

/**
Expand Down Expand Up @@ -1614,6 +1626,20 @@ class DocBaseViewer extends BaseViewer {
handleAnnotationCreatorChangeEvent({ status, type }) {
this.processAnnotationModeChange(this.annotationControlsFSM.transition(status, type));
}

updateDiscoverabilityResinTag() {
if (!this.containerEl) {
return;
}

const controlsState = this.annotationControlsFSM.getState();
const isDiscoverable = DISCOVERABILITY_STATES.includes(controlsState);
const isUsingDiscoverability = this.options.enableAnnotationsDiscoverability && isDiscoverable;

// For tracking purposes, set property to true when the annotation controls are in a state
// in which the default discoverability experience is enabled
this.containerEl.setAttribute(DISCOVERABILITY_ATTRIBUTE, isUsingDiscoverability);
}
}

export default DocBaseViewer;
58 changes: 56 additions & 2 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* eslint-disable no-unused-expressions */
import Api from '../../../api';
import AnnotationControls, { AnnotationMode } from '../../../AnnotationControls';
import DocBaseViewer from '../DocBaseViewer';
import AnnotationControlsFSM, { AnnotationInput, AnnotationState } from '../../../AnnotationControlsFSM';
import DocBaseViewer, { DISCOVERABILITY_STATES } from '../DocBaseViewer';
import DocFindBar from '../DocFindBar';
import Browser from '../../../Browser';
import BaseViewer from '../../BaseViewer';
Expand All @@ -10,7 +11,6 @@ import PageControls from '../../../PageControls';
import ZoomControls from '../../../ZoomControls';
import fullscreen from '../../../Fullscreen';
import DocPreloader from '../DocPreloader';
import { AnnotationInput } from '../../../AnnotationControlsFSM';
import * as file from '../../../file';
import * as util from '../../../util';

Expand Down Expand Up @@ -130,6 +130,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {

docBase.containerEl = containerEl;
docBase.rootEl = rootEl;
docBase.options.enableAnnotationsDiscoverability = true;
docBase.setup();

expect(docBase.docEl.classList.contains('bp-doc')).toBe(true);
Expand All @@ -142,6 +143,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
expect(docBase.thumbnailsSidebarEl.parentNode).toBe(docBase.containerEl.parentNode);

expect(docBase.loadTimeout).toBe(LOAD_TIMEOUT_MS);
expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
});

test('should not set a thumbnails sidebar element if the option is not enabled', () => {
Expand Down Expand Up @@ -2815,18 +2817,22 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {

expect(docBase.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
expect(docBase.processAnnotationModeChange).toBeCalledWith(AnnotationMode.NONE);
expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
});

test('should set annotations mode to none', () => {
docBase.annotator = {
toggleAnnotationMode: jest.fn(),
};
docBase.options.enableAnnotationsDiscoverability = false;
docBase.processAnnotationModeChange = jest.fn();
docBase.containerEl.setAttribute('data-resin-discoverability', false);

docBase.handleAnnotationControlsEscape();

expect(docBase.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.NONE);
expect(docBase.processAnnotationModeChange).not.toBeCalled();
expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('false');
});
});

Expand All @@ -2849,10 +2855,12 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
docBase.options.enableAnnotationsDiscoverability = false;
docBase.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(docBase.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.NONE);
expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('false');

docBase.options.enableAnnotationsDiscoverability = true;
docBase.handleAnnotationControlsClick({ mode: AnnotationMode.NONE });
expect(docBase.annotator.toggleAnnotationMode).toBeCalledWith(AnnotationMode.REGION);
expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
});
});

Expand All @@ -2869,5 +2877,51 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
},
);
});

describe('updateDiscoverabilityResinTag()', () => {
const REMAINING_STATES = Object.values(AnnotationState).filter(
state => !DISCOVERABILITY_STATES.includes(state),
);

beforeEach(() => {
docBase.containerEl = document.createElement('div');
});

test.each(Object.values(AnnotationState))(
'should set resin tag to false if enableAnnotationsDiscoverability is false even if state=%s',
state => {
docBase.options.enableAnnotationsDiscoverability = false;
docBase.annotationControlsFSM = new AnnotationControlsFSM(state);

docBase.updateDiscoverabilityResinTag();

expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('false');
},
);

test.each(REMAINING_STATES)(
'should set resin tag to false if enableAnnotationsDiscoverability is true but state is %s',
state => {
docBase.options.enableAnnotationsDiscoverability = true;
docBase.annotationControlsFSM = new AnnotationControlsFSM(state);

docBase.updateDiscoverabilityResinTag();

expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('false');
},
);

test.each(DISCOVERABILITY_STATES)(
'should set resin tag to true if enableDiscoverability is true and state is %s',
state => {
docBase.options.enableAnnotationsDiscoverability = true;
docBase.annotationControlsFSM = new AnnotationControlsFSM(state);

docBase.updateDiscoverabilityResinTag();

expect(docBase.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
},
);
});
});
});
Loading

0 comments on commit 480a90d

Please sign in to comment.