Skip to content

Commit

Permalink
feat(office): Preview PDF rep if using internet explorer (#1416)
Browse files Browse the repository at this point in the history
* feat(office): use PDF rep for IE11

* chore: add unit tests

* chore: pr comments

* chore: update OfficeLoader test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
Conrad Chan and mergify[bot] authored Aug 10, 2021
1 parent 75ede5e commit c104cd2
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ error_bad_file=We’re sorry the preview didn’t load. This file could not be c
error_not_downloadable=Oops! It looks like something is wrong with this file. We apologize for the inconvenience and recommend that you upload a new version of this file or roll back to a previous version. Please contact us for more details.
# Preview error message shown when flash is not enabled on their browser
error_flash_not_enabled=We’re sorry, the preview didn’t load because Flash is not enabled on your browser. If possible, please enable Flash and refresh the page.
# Preview error message shown when loading what would be an Office Excel Online file extension in Internet Explorer
error_internet_explorer_office_online=Microsoft 365 apps and services no longer support Internet Explorer 11, therefore, Box users may have a degraded experience.

# Archive Preview
# Label for filename column name
Expand Down
8 changes: 8 additions & 0 deletions src/lib/Browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,14 @@ class Browser {
},
};
}

/**
* Returns whether the browser is Internet Explorer
* @returns {boolean} - Whether the browser is Internet Explorer
*/
static isIE() {
return Browser.getName() === BROWSERS.INTERNET_EXPLORER;
}
}

export default Browser;
16 changes: 16 additions & 0 deletions src/lib/viewers/doc/DocumentViewer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Browser from '../../Browser';
import DocBaseViewer from './DocBaseViewer';
import DocPreloader from './DocPreloader';
import fullscreen from '../../Fullscreen';
import { EXCEL_EXTENSIONS } from '../../extensions';
import './Document.scss';

class DocumentViewer extends DocBaseViewer {
Expand Down Expand Up @@ -33,6 +35,20 @@ class DocumentViewer extends DocBaseViewer {
this.preloader.removeAllListeners('preload');
}

/**
* @inheritdoc
*/
load() {
super.load();

const { extension } = this.options.file;
const isExcelExtension = EXCEL_EXTENSIONS.includes(extension);

if (isExcelExtension && Browser.isIE()) {
this.options.ui.showNotification(__('error_internet_explorer_office_online'), null, true);
}
}

/**
* Handles keyboard events for document viewer.
*
Expand Down
74 changes: 74 additions & 0 deletions src/lib/viewers/doc/__tests__/DocumentViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/* eslint-disable no-unused-expressions */
import Api from '../../../api';
import Browser from '../../../Browser';
import DocumentViewer from '../DocumentViewer';
import DocBaseViewer from '../DocBaseViewer';
import BaseViewer from '../../BaseViewer';
Expand All @@ -15,6 +17,8 @@ describe('lib/viewers/doc/DocumentViewer', () => {
beforeEach(() => {
fixture.load('viewers/doc/__tests__/DocumentViewer-test.html');

stubs.api = new Api();

containerEl = document.querySelector('.container');
doc = new DocumentViewer({
container: containerEl,
Expand Down Expand Up @@ -83,6 +87,76 @@ describe('lib/viewers/doc/DocumentViewer', () => {
});
});

describe('load()', () => {
const docBaseLoadFunc = DocBaseViewer.prototype.load;

beforeEach(() => {
jest.spyOn(stubs.api, 'get').mockImplementation();
jest.spyOn(doc, 'setup').mockImplementation();
Object.defineProperty(DocBaseViewer.prototype, 'load', {
value: jest.fn().mockImplementation(() => Promise.resolve()),
});
jest.spyOn(doc, 'createContentUrlWithAuthParams').mockImplementation();
jest.spyOn(doc, 'handleAssetAndRepLoad').mockImplementation();
jest.spyOn(doc, 'getRepStatus').mockReturnValue({ getPromise: () => Promise.resolve() });
jest.spyOn(doc, 'loadAssets').mockImplementation();
jest.spyOn(doc, 'loadBoxAnnotations').mockImplementation();
});

afterEach(() => {
Object.defineProperty(DocBaseViewer.prototype, 'load', { value: docBaseLoadFunc });
});

test.each(['xls', 'xlsm', 'xlsx'])(
'should show notification if file has excel extension %s and is internet explorer',
extension => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = extension;
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);

doc.load();

expect(showNotification).toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
},
);

test.each(['xls', 'xlsm', 'xlsx'])(
'should not show notification if file has excel extension %s but is not internet explorer',
extension => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = extension;
jest.spyOn(Browser, 'isIE').mockImplementation(() => false);

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
},
);

test('should not show notification if file is not excel extension', () => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = 'pdf';

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
});

test('should not show notification if file is not excel extension and is internet explorer', () => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = 'pdf';
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
});
});

describe('onKeydown()', () => {
beforeEach(() => {
stubs.zoomIn = jest.spyOn(doc, 'zoomIn').mockImplementation();
Expand Down
4 changes: 3 additions & 1 deletion src/lib/viewers/office/OfficeLoader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import getProp from 'lodash/get';
import AssetLoader from '../AssetLoader';
import Browser from '../../Browser';
import OfficeViewer from './OfficeViewer';
import { checkPermission } from '../../file';
import { ORIGINAL_REP_NAME, PERMISSION_DOWNLOAD } from '../../constants';
Expand Down Expand Up @@ -39,7 +40,8 @@ class OfficeLoader extends AssetLoader {
if (
!checkPermission(file, PERMISSION_DOWNLOAD) ||
file.size > maxFileSize ||
isDisabledDueToPasswordProtectedSharedLink
isDisabledDueToPasswordProtectedSharedLink ||
Browser.isIE() // Disable Office viewer if browser is Internet Explorer due to EOL
) {
disabledViewers.push(OFFICE_VIEWER_NAME);
}
Expand Down
9 changes: 8 additions & 1 deletion src/lib/viewers/office/__tests__/OfficeLoader-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as file from '../../../file';
import Browser from '../../../Browser';
import OfficeLoader from '../OfficeLoader';
import OfficeViewer from '../OfficeViewer';
import * as file from '../../../file';

const FIVE_MB = 5242880;

Expand Down Expand Up @@ -86,6 +87,12 @@ describe('lib/viewers/office/OfficeLoader', () => {
const viewer = OfficeLoader.determineViewer(editedFakeFile, [], viewerOptions);
expect(viewer).toBeDefined();
});

test('should not return a viewer if the browser is internet explorer', () => {
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);
const viewer = OfficeLoader.determineViewer(fakeFile, []);
expect(viewer).toBeUndefined();
});
});
});
});

0 comments on commit c104cd2

Please sign in to comment.