Skip to content

Commit

Permalink
feat(annotations): Disable annotations for excel and iWork formats
Browse files Browse the repository at this point in the history
  • Loading branch information
Mingze Xiao committed May 6, 2020
1 parent 976fa7b commit 984fdeb
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 12 deletions.
14 changes: 8 additions & 6 deletions src/lib/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,16 @@ export const CODE_EXTENSIONS = [
'yaml',
];

// Should not include 'xlsb' cause xlsb conversion to pdf is not supported
// However, office viewer supports xlsb, xlsm, and xlsx (new formats), but not xls (old)
export const EXCEL_EXTENSIONS = ['xls', 'xlsm', 'xlsx'];

export const IWORK_EXTENSIONS = ['pages', 'numbers', 'key'];

export const DOCUMENT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS)
.concat(HTML_EXTENSIONS)
.concat(EXCEL_EXTENSIONS)
.concat(IWORK_EXTENSIONS)
.concat([
'doc',
'docx',
Expand All @@ -58,9 +66,6 @@ export const DOCUMENT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS)
'gsheet',
'gslide',
'gslides',
'pages',
'numbers',
'key',
'msg',
'odp',
'ods',
Expand All @@ -70,9 +75,6 @@ export const DOCUMENT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS)
'pptx',
'rtf',
'wpd',
'xls',
'xlsm',
'xlsx',
]);

export const TXT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS);
Expand Down
34 changes: 30 additions & 4 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
} from '../constants';
import { getIconFromExtension, getIconFromName } from '../icons/icons';
import { VIEWER_EVENT, ERROR_CODE, LOAD_METRIC, DOWNLOAD_REACHABILITY_METRICS } from '../events';
import { EXCEL_EXTENSIONS, IWORK_EXTENSIONS } from '../extensions';
import { AnnotationMode } from '../AnnotationControls';
import PreviewError from '../PreviewError';
import Timer from '../Timer';
Expand Down Expand Up @@ -548,7 +549,7 @@ class BaseViewer extends EventEmitter {
handleFullscreenEnter() {
this.resize();

if (this.annotator && this.options.showAnnotationsControls && this.annotationControls) {
if (this.areNewAnnotationsEnabled() && this.annotator && this.annotationControls) {
this.annotator.emit(ANNOTATOR_EVENT.setVisibility, false);

this.annotator.toggleAnnotationMode(AnnotationMode.NONE);
Expand All @@ -563,7 +564,7 @@ class BaseViewer extends EventEmitter {
*/
handleFullscreenExit() {
this.resize();
if (this.annotator && this.options.showAnnotationsControls) {
if (this.areNewAnnotationsEnabled() && this.annotator) {
this.annotator.emit(ANNOTATOR_EVENT.setVisibility, true);
}
}
Expand Down Expand Up @@ -976,7 +977,7 @@ class BaseViewer extends EventEmitter {
// Add a custom listener for events emmited by the annotator
this.annotator.addListener('annotatorevent', this.handleAnnotatorEvents);

if (this.options.showAnnotationsControls && this.annotationControls) {
if (this.areNewAnnotationsEnabled() && this.annotationControls) {
this.annotator.addListener('annotations_create', this.handleAnnotationCreateEvent);
}
}
Expand Down Expand Up @@ -1011,6 +1012,13 @@ class BaseViewer extends EventEmitter {
return false;
}

const { showAnnotations, showAnnotationsControls } = this.options;

// If it's new annotations experience
if (showAnnotations && showAnnotationsControls && !this.areNewAnnotationsEnabled()) {
return false;
}

// Respect viewer-specific annotation option if it is set
if (
window.BoxAnnotations &&
Expand All @@ -1032,7 +1040,25 @@ class BaseViewer extends EventEmitter {

// Ignore viewer config if BoxAnnotations was pass into Preview as an option
// Otherwise, use global preview annotation option
return !!this.options.showAnnotations;
return !!showAnnotations;
}

/**
* Returns whether or not new annotations are enabled for this viewer.
* If enabled, a new annotations button will show up in toolbar
*
* @return {boolean} Whether or not viewer enables new annotations
*/
areNewAnnotationsEnabled() {
const { showAnnotationsControls, file } = this.options;
const { extension } = file || {};

// Disable new annotations for Excel and iWork formats
if (EXCEL_EXTENSIONS.includes(extension) || IWORK_EXTENSIONS.includes(extension)) {
return false;
}

return showAnnotationsControls;
}

/**
Expand Down
27 changes: 27 additions & 0 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as util from '../../util';
import * as icons from '../../icons/icons';
import * as constants from '../../constants';
import { VIEWER_EVENT, LOAD_METRIC, ERROR_CODE } from '../../events';
import { EXCEL_EXTENSIONS, IWORK_EXTENSIONS } from '../../extensions';
import { AnnotationMode } from '../../AnnotationControls';
import Timer from '../../Timer';
import Api from '../../api';
Expand Down Expand Up @@ -1199,6 +1200,13 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.areAnnotationsEnabled()).to.be.false;
});

it('should return false if new annotations is not enabled', () => {
base.options.showAnnotationsControls = true;
sandbox.stub(base, 'areNewAnnotationsEnabled').returns(false);

expect(base.areAnnotationsEnabled()).to.be.false;
});

it('should return true if viewer option is set to true', () => {
expect(base.areAnnotationsEnabled()).to.be.false;

Expand Down Expand Up @@ -1254,6 +1262,25 @@ describe('lib/viewers/BaseViewer', () => {
});
});

describe('areNewAnnotationsEnabled()', () => {
EXCEL_EXTENSIONS.concat(IWORK_EXTENSIONS).forEach(extension => {
it(`should return false if the file is ${extension} format`, () => {
base.options.file.extension = extension;
base.options.showAnnotationsControls = true;
expect(base.areNewAnnotationsEnabled()).to.be.false;
});
});

it('should return showAnnotationsControls if file is not excel nor iWork formats', () => {
base.options.file.extension = 'pdf';
base.options.showAnnotationsControls = true;
expect(base.areNewAnnotationsEnabled()).to.be.true;

base.options.showAnnotationsControls = false;
expect(base.areNewAnnotationsEnabled()).to.be.false;
});
});

describe('getViewerAnnotationsConfig()', () => {
it('should return an empty object if none options available', () => {
sandbox.stub(base, 'getViewerOption').returns(undefined);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ class DocBaseViewer extends BaseViewer {
this.controls = new Controls(this.containerEl);
this.pageControls = new PageControls(this.controls, this.docEl);
this.zoomControls = new ZoomControls(this.controls);
if (this.options.showAnnotationsControls) {
if (this.areNewAnnotationsEnabled()) {
this.annotationControls = new AnnotationControls(this.controls);
}
this.pageControls.addListener('pagechange', this.setPage);
Expand Down Expand Up @@ -1100,7 +1100,7 @@ class DocBaseViewer extends BaseViewer {
);
this.controls.add(__('exit_fullscreen'), this.toggleFullscreen, 'bp-exit-fullscreen-icon', ICON_FULLSCREEN_OUT);

if (this.options.showAnnotationsControls) {
if (this.areNewAnnotationsEnabled()) {
this.annotationControls.init({
onRegionClick: this.regionClickHandler,
});
Expand Down

0 comments on commit 984fdeb

Please sign in to comment.