Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(annotations): Hide annotation controls when fullscreen is active #1188

Merged
merged 6 commits into from
Mar 31, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/lib/AnnotationControls.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,8 @@
}
}
}

&.is-hidden {
display: none;
}
}
76 changes: 60 additions & 16 deletions src/lib/AnnotationControls.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import noop from 'lodash/noop';
import { ICON_REGION_COMMENT } from './icons/icons';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from './Controls';
import fullscreen from './Fullscreen';

export const CLASS_ANNOTATIONS_GROUP = 'bp-AnnotationControls-group';
export const CLASS_REGION_BUTTON = 'bp-AnnotationControls-regionBtn';

export const CLASS_BUTTON_ACTIVE = 'is-active';
export const CLASS_GROUP_HIDE = 'is-hidden';

export type RegionHandler = ({ isRegionActive, event }: { isRegionActive: boolean; event: MouseEvent }) => void;
export type Options = {
Expand All @@ -17,48 +20,89 @@ export default class AnnotationControls {
/** @property {Controls} - Controls object */
private controls: Controls;

/** @property {HTMLElement} - Controls element */
private controlsElement: HTMLElement;

/** @property {boolean} - Region comment mode active state */
private isRegionActive = false;

/**
* [constructor]
*
* @param {Controls} controls - Viewer controls
* @return {AnnotationControls} Instance of AnnotationControls
*/
constructor(controls: Controls) {
if (!controls || !(controls instanceof Controls)) {
throw Error('controls must be an instance of Controls');
}

this.controls = controls;
this.controlsElement = controls.controlsEl;

this.attachEventHandlers();
}

/**
* [destructor]
*/
public destroy(): void {
fullscreen.removeListener('enter', this.handleFullscreenEnter);
fullscreen.removeListener('exit', this.handleFullscreenExit);
}

/**
* Attaches event handlers
*/
private attachEventHandlers(): void {
fullscreen.addListener('enter', this.handleFullscreenEnter);
fullscreen.addListener('exit', this.handleFullscreenExit);
}

/**
* Handle fullscreen change
*/
private handleFullscreenChange = (isFullscreen: boolean): void => {
const groupElement = this.controlsElement.querySelector(`.${CLASS_ANNOTATIONS_GROUP}`);

if (!groupElement) {
return;
}

if (isFullscreen) {
groupElement.classList.add(CLASS_GROUP_HIDE);
} else {
groupElement.classList.remove(CLASS_GROUP_HIDE);
}
};

/**
* Enter fullscreen handler
mxiao6 marked this conversation as resolved.
Show resolved Hide resolved
*/
private handleFullscreenEnter = (): void => this.handleFullscreenChange(true);

/**
* Exit fullscreen handler
*/
private handleFullscreenExit = (): void => this.handleFullscreenChange(false);

/**
* Region comment button click handler
*
* @param {RegionHandler} onRegionClick - region click handler in options
* @param {MouseEvent} event - mouse event
* @return {void}
*/
private handleRegionClick = (onRegionClick: RegionHandler) => (event: MouseEvent): void => {
const regionButtonElement = event.target as HTMLButtonElement;
const regionButtonElement = this.controlsElement.querySelector(`.${CLASS_REGION_BUTTON}`);

this.isRegionActive = !this.isRegionActive;
if (this.isRegionActive) {
regionButtonElement.classList.add(CLASS_BUTTON_ACTIVE);
} else {
regionButtonElement.classList.remove(CLASS_BUTTON_ACTIVE);
if (regionButtonElement) {
this.isRegionActive = !this.isRegionActive;
if (this.isRegionActive) {
regionButtonElement.classList.add(CLASS_BUTTON_ACTIVE);
} else {
regionButtonElement.classList.remove(CLASS_BUTTON_ACTIVE);
}
Comment on lines +92 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the toggleAnnotationMode api is setup with the corresponding event, we may want to consider refactoring this to respond to the event instead so we can remove the local state maintained by AnnotationControls

}

onRegionClick({ isRegionActive: this.isRegionActive, event });
};

/**
* Initialize the annotation controls with options.
*
* @param {RegionHandler} [options.onRegionClick] - Callback when region comment button is clicked
* @return {void}
*/
public init({ onRegionClick = noop }: Options = {}): void {
const groupElement = this.controls.addGroup(CLASS_ANNOTATIONS_GROUP);
Expand Down
65 changes: 49 additions & 16 deletions src/lib/__tests__/AnnotationControls-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
/* eslint-disable no-unused-expressions */
import AnnotationControls, { CLASS_REGION_BUTTON, CLASS_BUTTON_ACTIVE } from '../AnnotationControls';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from '../Controls';
import { ICON_REGION_COMMENT } from '../icons/icons';
import AnnotationControls, {
CLASS_ANNOTATIONS_GROUP,
CLASS_BUTTON_ACTIVE,
CLASS_GROUP_HIDE,
CLASS_REGION_BUTTON,
} from '../AnnotationControls';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from '../Controls';
import fullscreen from '../Fullscreen';

let annotationControls;
let stubs = {};
Expand All @@ -16,8 +22,20 @@ describe('lib/AnnotationControls', () => {
beforeEach(() => {
fixture.load('__tests__/AnnotationControls-test.html');
const controls = new Controls(document.getElementById('test-annotation-controls-container'));
annotationControls = new AnnotationControls(controls);

stubs.classListAdd = sandbox.stub();
stubs.classListRemove = sandbox.stub();
stubs.fullscreenAddListener = sandbox.stub(fullscreen, 'addListener');
stubs.onRegionClick = sandbox.stub();
stubs.querySelector = sandbox.stub().returns({
classList: {
add: stubs.classListAdd,
remove: stubs.classListRemove,
},
});

annotationControls = new AnnotationControls(controls);
annotationControls.controlsElement.querySelector = stubs.querySelector;
});

afterEach(() => {
Expand All @@ -33,11 +51,27 @@ describe('lib/AnnotationControls', () => {
expect(annotationControls.controls).not.to.be.undefined;
});

it('should attach event listeners', () => {
expect(stubs.fullscreenAddListener).to.be.calledTwice;
});

it('should throw an exception if controls is not provided', () => {
expect(() => new AnnotationControls()).to.throw(Error, 'controls must be an instance of Controls');
});
});

describe('destroy()', () => {
beforeEach(() => {
stubs.fullscreenRemoveListener = sandbox.stub(fullscreen, 'removeListener');
});

it('should remove all listeners', () => {
annotationControls.destroy();

expect(stubs.fullscreenRemoveListener).to.be.calledTwice;
});
});

describe('init()', () => {
beforeEach(() => {
stubs.add = sandbox.stub(annotationControls.controls, 'add');
Expand All @@ -60,19 +94,6 @@ describe('lib/AnnotationControls', () => {
});

describe('handleRegionClick()', () => {
beforeEach(() => {
stubs.classListAdd = sandbox.stub();
stubs.classListRemove = sandbox.stub();
stubs.event = sandbox.stub({
target: {
classList: {
add: stubs.classListAdd,
remove: stubs.classListRemove,
},
},
});
});

it('should activate region button then deactivate', () => {
expect(annotationControls.isRegionActive).to.be.false;

Expand All @@ -94,4 +115,16 @@ describe('lib/AnnotationControls', () => {
});
});
});

describe('handleFullscreenChange()', () => {
it('should hide entire group if fullscreen is active', () => {
annotationControls.handleFullscreenEnter();
expect(stubs.querySelector).to.be.calledWith(`.${CLASS_ANNOTATIONS_GROUP}`);
expect(stubs.classListAdd).to.be.calledWith(CLASS_GROUP_HIDE);

annotationControls.handleFullscreenExit();
expect(stubs.querySelector).to.be.calledWith(`.${CLASS_ANNOTATIONS_GROUP}`);
expect(stubs.classListRemove).to.be.calledWith(CLASS_GROUP_HIDE);
});
});
});
4 changes: 4 additions & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ class DocBaseViewer extends BaseViewer {
URL.revokeObjectURL(this.printURL);
}

if (this.annotationControls) {
this.annotationControls.destroy();
}

if (this.pageControls) {
this.pageControls.removeListener('pagechange', this.setPage);
}
Expand Down