Skip to content

Commit

Permalink
chore: Differentiate discoverability between image and doc
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan committed Oct 26, 2020
1 parent ba485cd commit 2dfadfb
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 92 deletions.
19 changes: 13 additions & 6 deletions src/lib/AnnotationControlsFSM.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ export enum AnnotationInput {
UPDATE = 'update',
}

export const DISCOVERABILITY_STATES = [
AnnotationState.HIGHLIGHT_TEMP,
AnnotationState.NONE,
AnnotationState.REGION_TEMP,
];
type Subscription = (state: AnnotationState) => void;

export const modeStateMap = {
[AnnotationMode.HIGHLIGHT]: AnnotationState.HIGHLIGHT,
Expand All @@ -47,22 +43,28 @@ 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 isDiscoverable = (): boolean => DISCOVERABILITY_STATES.includes(this.currentState);
private notify = (): void => this.subscriptions.forEach(callback => callback(this.currentState));

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

public transition = (input: AnnotationInput, mode: AnnotationMode = AnnotationMode.NONE): AnnotationMode => {
if (input === AnnotationInput.CLICK) {
this.currentState = mode === stateModeMap[this.currentState] ? AnnotationState.NONE : modeStateMap[mode];
this.notify();
return stateModeMap[this.currentState];
}

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

Expand All @@ -81,6 +83,11 @@ export default class AnnotationControlsFSM {
default:
}

this.notify();
return stateModeMap[this.currentState];
};

public unsubscribe = (callback: Subscription): void => {
this.subscriptions = this.subscriptions.filter(subscription => subscription !== callback);
};
}
35 changes: 20 additions & 15 deletions src/lib/__tests__/AnnotationControlsFSM-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import AnnotationControlsFSM, {
AnnotationInput,
AnnotationState,
DISCOVERABILITY_STATES,
} from '../AnnotationControlsFSM';
import AnnotationControlsFSM, { AnnotationInput, AnnotationState } from '../AnnotationControlsFSM';
import { AnnotationMode } from '../AnnotationControls';

describe('lib/AnnotationControlsFSM', () => {
Expand Down Expand Up @@ -270,19 +266,28 @@ describe('lib/AnnotationControlsFSM', () => {
});
});

describe('isDiscoverable()', () => {
const REMAINING_STATES = Object.values(AnnotationState).filter(
state => !DISCOVERABILITY_STATES.includes(state),
);
describe('subscriptions', () => {
const fsm = new AnnotationControlsFSM();
let callback;

test.each(REMAINING_STATES)('should return false if state is %s', state => {
const fsm = new AnnotationControlsFSM(state);
expect(fsm.isDiscoverable()).toBe(false);
beforeEach(() => {
callback = jest.fn();
fsm.subscribe(callback);
});

test.each(DISCOVERABILITY_STATES)('should return true if state is %s', state => {
const fsm = new AnnotationControlsFSM(state);
expect(fsm.isDiscoverable()).toBe(true);
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());
});
});
});
15 changes: 0 additions & 15 deletions src/lib/viewers/BaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,9 @@ class BaseViewer extends EventEmitter {
this.containerEl.classList.add(CLASS_ANNOTATIONS_DISCOVERABLE);
}

this.updateDiscoverabilityResinTag();

this.isSetup = true;
}

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

const isDiscoverable = this.annotationControlsFSM.isDiscoverable();
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('data-resin-discoverability', isUsingDiscoverability);
}

/**
* Removes the crawler and sets the file type specific loading icon
*
Expand Down
48 changes: 0 additions & 48 deletions src/lib/viewers/__tests__/BaseViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import EventEmitter from 'events';
import * as constants from '../../constants';
import * as icons from '../../icons/icons';
import * as util from '../../util';
import AnnotationControlsFSM, { AnnotationState, DISCOVERABILITY_STATES } from '../../AnnotationControlsFSM';
import Api from '../../api';
import BaseViewer from '../BaseViewer';
import Browser from '../../Browser';
Expand Down Expand Up @@ -85,7 +84,6 @@ describe('lib/viewers/BaseViewer', () => {
expect(typeof base.loadTimeout).toBe('number');
expect(base.annotatorPromise).toBeDefined();
expect(base.annotatorPromiseResolver).toBeDefined();
expect(base.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
});

test('should add a mobile class to the container if on mobile', () => {
Expand Down Expand Up @@ -1818,50 +1816,4 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.getInitialAnnotationMode()).toBe(AnnotationMode.NONE);
});
});

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

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

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

base.updateDiscoverabilityResinTag();

expect(base.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 => {
base.options.enableAnnotationsDiscoverability = true;
base.annotationControlsFSM = new AnnotationControlsFSM(state);

base.updateDiscoverabilityResinTag();

expect(base.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 => {
base.options.enableAnnotationsDiscoverability = true;
base.annotationControlsFSM = new AnnotationControlsFSM(state);

base.updateDiscoverabilityResinTag();

expect(base.containerEl.getAttribute('data-resin-discoverability')).toBe('true');
},
);
});
});
34 changes: 29 additions & 5 deletions src/lib/viewers/doc/DocBaseViewer.js
View file
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 Down Expand Up @@ -48,6 +48,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 @@ -94,6 +100,8 @@ class DocBaseViewer extends BaseViewer {
constructor(options) {
super(options);

this.annotationControlsFSM.subscribe(this.handleFSMChange);

// Bind context for callbacks
this.emitMetric = this.emitMetric.bind(this);
this.handleAssetAndRepLoad = this.handleAssetAndRepLoad.bind(this);
Expand Down Expand Up @@ -158,6 +166,8 @@ class DocBaseViewer extends BaseViewer {
this.thumbnailsSidebarEl.tabIndex = 0;
this.rootEl.insertBefore(this.thumbnailsSidebarEl, this.containerEl);
}

this.updateDiscoverabilityResinTag();
}

/**
Expand Down Expand Up @@ -1590,8 +1600,6 @@ class DocBaseViewer extends BaseViewer {
: nextMode,
);
this.processAnnotationModeChange(nextMode);

this.updateDiscoverabilityResinTag();
}

handleAnnotationControlsEscape() {
Expand All @@ -1601,8 +1609,6 @@ class DocBaseViewer extends BaseViewer {
} else {
this.annotator.toggleAnnotationMode(AnnotationMode.NONE);
}

this.updateDiscoverabilityResinTag();
}

handleAnnotationCreateEvent({ annotation: { id } = {}, meta: { status } = {} }) {
Expand All @@ -1618,6 +1624,24 @@ class DocBaseViewer extends BaseViewer {
handleAnnotationCreatorChangeEvent({ status, type }) {
this.processAnnotationModeChange(this.annotationControlsFSM.transition(status, type));
}

handleFSMChange = () => {
this.updateDiscoverabilityResinTag();
};

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('data-resin-discoverability', isUsingDiscoverability);
}
}

export default DocBaseViewer;
53 changes: 51 additions & 2 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
View file
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 @@ -2824,6 +2826,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
};
docBase.options.enableAnnotationsDiscoverability = false;
docBase.processAnnotationModeChange = jest.fn();
docBase.containerEl.setAttribute('data-resin-discoverability', false);

docBase.handleAnnotationControlsEscape();

Expand Down Expand Up @@ -2874,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 2dfadfb

Please sign in to comment.