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): Enable annotations for non-spreadsheet iWork files #1227

Merged
merged 2 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 5 additions & 4 deletions src/lib/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,14 @@ export const CODE_EXTENSIONS = [
'yaml',
];

// Should not include 'xlsb' cause xlsb conversion to pdf is not supported
// Should not include 'xlsb', since 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 EXCLUDED_EXTENSIONS = EXCEL_EXTENSIONS.concat(['numbers']);

export const DOCUMENT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS)
.concat(HTML_EXTENSIONS)
.concat(EXCEL_EXTENSIONS)
.concat(IWORK_EXTENSIONS)
.concat([
'doc',
'docx',
Expand All @@ -66,10 +64,13 @@ export const DOCUMENT_EXTENSIONS = CODE_EXTENSIONS.concat(NON_CODE_EXTENSIONS)
'gsheet',
'gslide',
'gslides',
'key',
'msg',
'numbers',
'odp',
'ods',
'odt',
'pages',
'pdf',
'ppt',
'pptx',
Expand Down
6 changes: 3 additions & 3 deletions src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ import {
STATUS_SUCCESS,
STATUS_VIEWABLE,
} from '../constants';
import { EXCLUDED_EXTENSIONS } from '../extensions';
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 @@ -1140,8 +1140,8 @@ class BaseViewer extends EventEmitter {
return false;
}

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

Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/__tests__/BaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import intl from '../../i18n';
import * as util from '../../util';
import * as icons from '../../icons/icons';
import * as constants from '../../constants';
import { EXCLUDED_EXTENSIONS } from '../../extensions';
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 @@ -1463,7 +1463,7 @@ describe('lib/viewers/BaseViewer', () => {
expect(base.areNewAnnotationsEnabled()).to.be.false;
});

EXCEL_EXTENSIONS.concat(IWORK_EXTENSIONS).forEach(extension => {
EXCLUDED_EXTENSIONS.forEach(extension => {
it(`should return false if the file is ${extension} format`, () => {
base.options.file.extension = extension;
base.options.showAnnotationsControls = true;
Expand Down