Commit 3efd1862 authored by Robert Knight's avatar Robert Knight

Enable side-by-side mode for web pages behind a feature flag

Enable support for side-by-side mode for all web pages, behind an
`html_side_by_side` feature flag.

 - Add `html_side_by_side` flag to known flags in `features.js`

 - Define `FeatureFlags` interface for querying (but not updating flags)
   and make the `FeatureFlags` class implement it

 - Pass the `FeatureFlags` instance from Guest through to
   HTMLIntegration and read/observe the `html_side_by_side` flag to
   enable/disable side-by-side mode

 - Create a dummy `FeatureFlags` implementation in
   `VitalSourceContentIntegration` which always has the
   `html_side_by_side` flag turned on
parent beb10ec9
...@@ -2,15 +2,21 @@ import { TinyEmitter } from 'tiny-emitter'; ...@@ -2,15 +2,21 @@ import { TinyEmitter } from 'tiny-emitter';
import { warnOnce } from '../shared/warn-once'; import { warnOnce } from '../shared/warn-once';
/**
* @typedef {import('../types/annotator').FeatureFlags} IFeatureFlags
*/
/** /**
* List of feature flags that annotator code tests for. * List of feature flags that annotator code tests for.
* *
* @type {string[]} * @type {string[]}
*/ */
const annotatorFlags = []; const annotatorFlags = ['html_side_by_side'];
/** /**
* An observable container of feature flags. * An observable container of feature flags.
*
* @implements {IFeatureFlags}
*/ */
export class FeatureFlags extends TinyEmitter { export class FeatureFlags extends TinyEmitter {
/** /**
......
...@@ -9,6 +9,8 @@ import { scrollElementIntoView } from '../util/scroll'; ...@@ -9,6 +9,8 @@ import { scrollElementIntoView } from '../util/scroll';
/** /**
* @typedef {import('../../types/annotator').Anchor} Anchor * @typedef {import('../../types/annotator').Anchor} Anchor
* @typedef {import('../../types/annotator').Annotator} Annotator
* @typedef {import('../../types/annotator').FeatureFlags} FeatureFlags
* @typedef {import('../../types/annotator').Integration} Integration * @typedef {import('../../types/annotator').Integration} Integration
* @typedef {import('../../types/annotator').SidebarLayout} SidebarLayout * @typedef {import('../../types/annotator').SidebarLayout} SidebarLayout
*/ */
...@@ -27,20 +29,41 @@ const MIN_HTML_WIDTH = 480; ...@@ -27,20 +29,41 @@ const MIN_HTML_WIDTH = 480;
* @implements {Integration} * @implements {Integration}
*/ */
export class HTMLIntegration { export class HTMLIntegration {
constructor(container = document.body) { /**
* @param {object} options
* @param {FeatureFlags} options.features
* @param {HTMLElement} [options.container]
*/
constructor({ features, container = document.body }) {
this.features = features;
this.container = container; this.container = container;
this.anchor = anchor; this.anchor = anchor;
this.describe = describe; this.describe = describe;
this._htmlMeta = new HTMLMetadata(); this._htmlMeta = new HTMLMetadata();
/** Whether to attempt to resize the document to fit alongside sidebar. */
this._sideBySideEnabled = this.features.flagEnabled('html_side_by_side');
/** /**
* Whether to attempt to resize the document contents when {@link fitSideBySide} * Whether the document is currently being resized to fit alongside an
* is called. * open sidebar.
*
* Currently disabled by default.
*/ */
this.sideBySideEnabled = false; this._sideBySideActive = false;
/** @type {SidebarLayout|null} */
this._lastLayout = null;
this._flagsChanged = () => {
const sideBySide = features.flagEnabled('html_side_by_side');
if (sideBySide !== this._sideBySideEnabled) {
this._sideBySideEnabled = sideBySide;
if (this._lastLayout) {
this.fitSideBySide(this._lastLayout);
}
}
};
this.features.on('flagsChanged', this._flagsChanged);
} }
canAnnotate() { canAnnotate() {
...@@ -48,7 +71,7 @@ export class HTMLIntegration { ...@@ -48,7 +71,7 @@ export class HTMLIntegration {
} }
destroy() { destroy() {
// There is nothing to do here yet. this.features.off('flagsChanged', this._flagsChanged);
} }
contentContainer() { contentContainer() {
...@@ -59,20 +82,23 @@ export class HTMLIntegration { ...@@ -59,20 +82,23 @@ export class HTMLIntegration {
* @param {SidebarLayout} layout * @param {SidebarLayout} layout
*/ */
fitSideBySide(layout) { fitSideBySide(layout) {
const maximumWidthToFit = window.innerWidth - layout.width; this._lastLayout = layout;
const active = layout.expanded && maximumWidthToFit >= MIN_HTML_WIDTH;
if (!this.sideBySideEnabled) { const maximumWidthToFit = window.innerWidth - layout.width;
return false; const active =
} this._sideBySideEnabled &&
layout.expanded &&
maximumWidthToFit >= MIN_HTML_WIDTH;
if (active) { if (active) {
// nb. We call `_activateSideBySide` regardless of whether side-by-side
// is already active because the sidebar width might be different.
this._activateSideBySide(layout.width); this._activateSideBySide(layout.width);
return true; } else if (this._sideBySideActive) {
} else {
this._deactivateSideBySide(); this._deactivateSideBySide();
return false;
} }
this._sideBySideActive = active;
return active;
} }
/** /**
......
...@@ -31,5 +31,5 @@ export function createIntegration(annotator, { contentPartner } = {}) { ...@@ -31,5 +31,5 @@ export function createIntegration(annotator, { contentPartner } = {}) {
return new VitalSourceContentIntegration(); return new VitalSourceContentIntegration();
} }
return new HTMLIntegration(); return new HTMLIntegration({ features: annotator.features });
} }
import { FeatureFlags } from '../../features';
import { HTMLIntegration, $imports } from '../html'; import { HTMLIntegration, $imports } from '../html';
describe('HTMLIntegration', () => { describe('HTMLIntegration', () => {
let features;
let fakeHTMLAnchoring; let fakeHTMLAnchoring;
let fakeHTMLMetadata; let fakeHTMLMetadata;
let fakeGuessMainContentArea; let fakeGuessMainContentArea;
...@@ -8,6 +10,8 @@ describe('HTMLIntegration', () => { ...@@ -8,6 +10,8 @@ describe('HTMLIntegration', () => {
let fakeScrollElementIntoView; let fakeScrollElementIntoView;
beforeEach(() => { beforeEach(() => {
features = new FeatureFlags();
fakeHTMLAnchoring = { fakeHTMLAnchoring = {
anchor: sinon.stub(), anchor: sinon.stub(),
describe: sinon.stub(), describe: sinon.stub(),
...@@ -41,15 +45,19 @@ describe('HTMLIntegration', () => { ...@@ -41,15 +45,19 @@ describe('HTMLIntegration', () => {
$imports.$restore(); $imports.$restore();
}); });
function createIntegration() {
return new HTMLIntegration({ features });
}
it('implements `anchor` and `destroy` using HTML anchoring', () => { it('implements `anchor` and `destroy` using HTML anchoring', () => {
const integration = new HTMLIntegration(); const integration = createIntegration();
assert.equal(integration.anchor, fakeHTMLAnchoring.anchor); assert.equal(integration.anchor, fakeHTMLAnchoring.anchor);
assert.equal(integration.describe, fakeHTMLAnchoring.describe); assert.equal(integration.describe, fakeHTMLAnchoring.describe);
}); });
describe('#canAnnotate', () => { describe('#canAnnotate', () => {
it('is always true', () => { it('is always true', () => {
const integration = new HTMLIntegration(); const integration = createIntegration();
const range = new Range(); const range = new Range();
assert.isTrue(integration.canAnnotate(range)); assert.isTrue(integration.canAnnotate(range));
}); });
...@@ -57,14 +65,25 @@ describe('HTMLIntegration', () => { ...@@ -57,14 +65,25 @@ describe('HTMLIntegration', () => {
describe('#contentContainer', () => { describe('#contentContainer', () => {
it('returns body by default', () => { it('returns body by default', () => {
const integration = new HTMLIntegration(); const integration = createIntegration();
assert.equal(integration.contentContainer(), document.body); assert.equal(integration.contentContainer(), document.body);
}); });
}); });
describe('#destroy', () => { describe('#destroy', () => {
it('does nothing', () => { it('cleans up feature flag listeners', () => {
new HTMLIntegration().destroy(); sinon.spy(features, 'on');
sinon.spy(features, 'off');
const integration = createIntegration();
assert.called(features.on);
const listeners = features.on.args;
integration.destroy();
for (let [event, callback] of listeners) {
assert.calledWith(features.off, event, callback);
}
}); });
}); });
...@@ -98,12 +117,6 @@ describe('HTMLIntegration', () => { ...@@ -98,12 +117,6 @@ describe('HTMLIntegration', () => {
); );
} }
function createIntegration() {
const integration = new HTMLIntegration();
integration.sideBySideEnabled = true;
return integration;
}
beforeEach(() => { beforeEach(() => {
// By default, pretend that the content fills the page. // By default, pretend that the content fills the page.
fakeGuessMainContentArea.returns(fullWidthContentRect()); fakeGuessMainContentArea.returns(fullWidthContentRect());
...@@ -116,10 +129,15 @@ describe('HTMLIntegration', () => { ...@@ -116,10 +129,15 @@ describe('HTMLIntegration', () => {
}); });
it('does nothing when disabled', () => { it('does nothing when disabled', () => {
new HTMLIntegration().fitSideBySide({}); const integration = createIntegration();
integration.fitSideBySide({});
}); });
context('when enabled', () => { context('when enabled', () => {
beforeEach(() => {
features.update({ html_side_by_side: true });
});
it('sets left and right margins on body element when activated', () => { it('sets left and right margins on body element when activated', () => {
const integration = createIntegration(); const integration = createIntegration();
...@@ -269,11 +287,29 @@ describe('HTMLIntegration', () => { ...@@ -269,11 +287,29 @@ describe('HTMLIntegration', () => {
}); });
}); });
}); });
const isSideBySideActive = () => {
const [left, right] = getMargins();
return left !== null || right !== null;
};
it('side-by-side is enabled/disabled when feature flag changes', () => {
const integration = createIntegration();
integration.fitSideBySide({ expanded: true, width: sidebarWidth });
assert.isFalse(isSideBySideActive());
features.update({ html_side_by_side: true });
assert.isTrue(isSideBySideActive());
features.update({ html_side_by_side: false });
assert.isFalse(isSideBySideActive());
});
}); });
describe('#getMetadata', () => { describe('#getMetadata', () => {
it('returns document metadata', async () => { it('returns document metadata', async () => {
const integration = new HTMLIntegration(); const integration = createIntegration();
assert.deepEqual(await integration.getMetadata(), { assert.deepEqual(await integration.getMetadata(), {
title: 'Example site', title: 'Example site',
}); });
...@@ -295,7 +331,7 @@ describe('HTMLIntegration', () => { ...@@ -295,7 +331,7 @@ describe('HTMLIntegration', () => {
it('scrolls to first highlight of anchor', async () => { it('scrolls to first highlight of anchor', async () => {
const anchor = { highlights: [highlight] }; const anchor = { highlights: [highlight] };
const integration = new HTMLIntegration(); const integration = createIntegration();
await integration.scrollToAnchor(anchor); await integration.scrollToAnchor(anchor);
assert.calledOnce(fakeScrollElementIntoView); assert.calledOnce(fakeScrollElementIntoView);
...@@ -305,7 +341,7 @@ describe('HTMLIntegration', () => { ...@@ -305,7 +341,7 @@ describe('HTMLIntegration', () => {
it('does nothing if anchor has no highlights', async () => { it('does nothing if anchor has no highlights', async () => {
const anchor = {}; const anchor = {};
const integration = new HTMLIntegration(); const integration = createIntegration();
await integration.scrollToAnchor(anchor); await integration.scrollToAnchor(anchor);
assert.notCalled(fakeScrollElementIntoView); assert.notCalled(fakeScrollElementIntoView);
...@@ -314,7 +350,7 @@ describe('HTMLIntegration', () => { ...@@ -314,7 +350,7 @@ describe('HTMLIntegration', () => {
describe('#uri', () => { describe('#uri', () => {
it('returns main document URL', async () => { it('returns main document URL', async () => {
const integration = new HTMLIntegration(); const integration = createIntegration();
assert.deepEqual(await integration.uri(), 'https://example.com/'); assert.deepEqual(await integration.uri(), 'https://example.com/');
}); });
}); });
......
...@@ -212,9 +212,11 @@ describe('annotator/integrations/vitalsource', () => { ...@@ -212,9 +212,11 @@ describe('annotator/integrations/vitalsource', () => {
it('delegates to HTMLIntegration for side-by-side mode', () => { it('delegates to HTMLIntegration for side-by-side mode', () => {
const integration = createIntegration(); const integration = createIntegration();
fakeHTMLIntegration.fitSideBySide.returns(true); assert.calledOnce(FakeHTMLIntegration);
assert.isTrue(fakeHTMLIntegration.sideBySideEnabled); const htmlOptions = FakeHTMLIntegration.args[0][0];
assert.isTrue(htmlOptions.features.flagEnabled('html_side_by_side'));
fakeHTMLIntegration.fitSideBySide.returns(true);
const layout = { expanded: true, width: 150 }; const layout = { expanded: true, width: 150 };
const isActive = integration.fitSideBySide(layout); const isActive = integration.fitSideBySide(layout);
......
import { ListenerCollection } from '../../shared/listener-collection'; import { ListenerCollection } from '../../shared/listener-collection';
import { FeatureFlags } from '../features';
import { onDocumentReady } from '../frame-observer'; import { onDocumentReady } from '../frame-observer';
import { HTMLIntegration } from './html'; import { HTMLIntegration } from './html';
import { preserveScrollPosition } from './html-side-by-side'; import { preserveScrollPosition } from './html-side-by-side';
...@@ -206,8 +207,9 @@ export class VitalSourceContentIntegration { ...@@ -206,8 +207,9 @@ export class VitalSourceContentIntegration {
* @param {HTMLElement} container * @param {HTMLElement} container
*/ */
constructor(container = document.body) { constructor(container = document.body) {
this._htmlIntegration = new HTMLIntegration(container); const features = new FeatureFlags();
this._htmlIntegration.sideBySideEnabled = true; features.update({ html_side_by_side: true });
this._htmlIntegration = new HTMLIntegration({ container, features });
this._listeners = new ListenerCollection(); this._listeners = new ListenerCollection();
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
* Type definitions for objects passed between the annotator and sidebar. * Type definitions for objects passed between the annotator and sidebar.
*/ */
/** @typedef {import('tiny-emitter').TinyEmitter} TinyEmitter */
/** /**
* Object representing a region of a document that an annotation * Object representing a region of a document that an annotation
* has been anchored to. * has been anchored to.
...@@ -76,12 +78,22 @@ ...@@ -76,12 +78,22 @@
* @prop {(root: HTMLElement, range: Range, options: unknown) => Selector[]|Promise<Selector[]>} describe * @prop {(root: HTMLElement, range: Range, options: unknown) => Selector[]|Promise<Selector[]>} describe
*/ */
/**
* Interface for querying a collection of feature flags and subscribing to
* flag updates.
*
* Emits a "flagsChanged" event when the flags are updated.
*
* @typedef {TinyEmitter & { flagEnabled: (flag: string) => boolean }} FeatureFlags
*/
/** /**
* Subset of the `Guest` class that is exposed to integrations. * Subset of the `Guest` class that is exposed to integrations.
* *
* @typedef Annotator * @typedef Annotator
* @prop {Anchor[]} anchors * @prop {Anchor[]} anchors
* @prop {(ann: AnnotationData) => Promise<Anchor[]>} anchor * @prop {(ann: AnnotationData) => Promise<Anchor[]>} anchor
* @prop {FeatureFlags} features
*/ */
/** /**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment