Chore: Preview Cleanup (#202)

* Chore: Disable Excel Online platform viewer

* Chore: Preview cleanup

src/lib/Preview.js

@@ -157,7 +157,6 @@ const PREVIEW_LOCATION = findScriptLocation(PREVIEW_SCRIPT_NAME, document.curren
         if (typeof token === 'string' || typeof token === 'function') {
             this.previewOptions = Object.assign({}, options, { token });
         } else if (token) {
-            // @TODO(tjin): Remove this after expiring embed updates to new calling pattern
             this.previewOptions = Object.assign({}, token || {});
         } else {
             throw new Error('Missing access token!');
@@ -1201,8 +1200,6 @@ const PREVIEW_LOCATION = findScriptLocation(PREVIEW_SCRIPT_NAME, document.curren
     /**
      * Global keydown handler for preview.
      *
-     * @TODO fix multiple preview key issue
-     * @TODO fire key event
      *
      * @private
      * @param {Event} event - keydown event

src/lib/__tests__/Preview-test.js

@@ -1860,26 +1860,6 @@ describe('lib/Preview', () => {
             const loader = preview.getLoader('file');
             expect(loader.name).to.equal('csv');
         });
-
-        // it('should not rethrow errors by default', () => {
-        //     preview.loaders = [
-        //         {
-        //             canLoad: () => { throw new Error('test'); }
-        //         }
-        //     ];
-
-        //     expect(preview.getLoader('file')).to.be.null;
-        // });
-
-        // it('should throw errors if rethrow is true', () => {
-        //     preview.loaders = [
-        //         {
-        //             canLoad: () => { throw new Error('test'); }
-        //         }
-        //     ];
-
-        //     expect(() => { preview.getLoader('file', true); }).to.throw(Error, /test/);
-        // });
     });
 
     describe('keydownHandler()', () => {

src/lib/annotations/AnnotationService.js

@@ -89,7 +89,6 @@ const ANONYMOUS_USER = {
                 .then((response) => response.json())
                 .then((data) => {
                     if (data.type !== 'error' && data.id) {
-                        // @TODO(tjin): Remove this when response has permissions
                         const tempData = data;
                         tempData.permissions = {
                             can_edit: true,

src/lib/annotations/__tests__/AnnotationService-test.js

@@ -186,10 +186,6 @@ describe('lib/annotations/AnnotationService', () => {
         });
     });
 
-    describe('update()', () => {
-        // @TODO(tjin): Test when update() is implemented
-    });
-
     describe('delete()', () => {
         const url = `${API_HOST}/2.0/annotations/3`;
 
@@ -267,10 +263,6 @@ describe('lib/annotations/AnnotationService', () => {
         });
     });
 
-    describe('getAnnotationUser()', () => {
-        // @TODO(tjin): Test when getAnnotationUser() is updated after the release of transactional users
-    });
-
     describe('createThreadMap()', () => {
         it('should create a thread map with the correct annotations, in the correct order', () => {
             // Dates are provided as a string format from the API such as "2016-10-30T14:19:56",

src/lib/viewers/BaseViewer.js

@@ -610,7 +610,6 @@ const RESIZE_WAIT_TIME_IN_MILLIS = 300;
 
     /**
      * Orient annotations to the correct scale and orientation of the annotated document.
-     * @TODO(jholdstock|spramod): Remove this once we are emitting the correct messaging.
      *
      * @protected
      * @param {Object} data - Scale and orientation values needed to orient annotations.

src/lib/viewers/office/OfficeViewer.js

@@ -11,7 +11,6 @@ const LOAD_TIMEOUT_MS = 120000;
 const SAFARI_PRINT_TIMEOUT_MS = 1000; // Wait 1s before trying to print
 const PRINT_DIALOG_TIMEOUT_MS = 500;
 
-// @TODO(jpress): replace with discovery and XML parsing once Microsoft allows CORS
 const EXCEL_ONLINE_EMBED_URL = 'https://excel.officeapps.live.com/x/_layouts/xlembed.aspx';
 const OFFICE_ONLINE_IFRAME_NAME = 'office-online-iframe';
 const MESSAGE_HOST_READY = 'Host_PostmessageReady';
@@ -29,10 +28,8 @@ const MESSAGE_HOST_READY = 'Host_PostmessageReady';
 
         // Call super() to set up common layout
         super.setup();
-        // Set to false only in the WebApp, everywhere else we want to avoid hitting a runmode.
-        // This flag will be removed once we run the entire integration through the client.
-        const hasSetupOption = this.options.viewers.Office && 'shouldUsePlatformSetup' in this.options.viewers.Office;
-        this.platformSetup = hasSetupOption ? !!this.options.viewers.Office.shouldUsePlatformSetup : true;
+
+        this.platformSetup = false;
         this.setupIframe();
         this.initPrint();
         this.setupPDFUrl();
@@ -183,7 +180,6 @@ const MESSAGE_HOST_READY = 'Host_PostmessageReady';
      * @return {string} Runmode URL
      */
     setupRunmodeURL(appHost, fileId, sharedLink) {
-        // @TODO(jpress): Combine with setupWopiSrc Logic when removing the platform fork
         let route = '/integrations/officeonline/openExcelOnlinePreviewer';
 
         const domain = document.createElement('a');
@@ -220,7 +216,6 @@ const MESSAGE_HOST_READY = 'Host_PostmessageReady';
      * @return {string} WOPI src URL
      */
     setupWOPISrc(apiHost, fileId, sharedLink) {
-        // @TODO(jpress): add support for vanity URLs once WOPI API is updated
         let wopiSrc = `${apiHost}/wopi/files/`;
 
         if (sharedLink) {
@@ -272,7 +267,6 @@ const MESSAGE_HOST_READY = 'Host_PostmessageReady';
         const WOPISrc = this.setupWOPISrc(apiHost, fileId, sharedLink);
         const origin = { origin: window.location.origin };
         const formEl = this.containerEl.appendChild(document.createElement('form'));
-        // @TODO(jpress): add suport for iframe performance logging via App_LoadingStatus message
         // We pass our origin in the sessionContext so that Microsoft will pass
         // this to the checkFileInfo endpoint. From their we can set it as the
         // origin for iframe postMessage communications.

src/lib/viewers/office/__tests__/OfficeViewer-test.js

@@ -80,29 +80,9 @@ describe('lib/viewers/office/OfficeViewer', () => {
             expect(stubs.setupPDFUrl).to.be.called;
         });
 
-        it('should not determine setup based on the option if it is passed in', () => {
+        it('should not use platform setup', () => {
             office.setup();
             expect(office.platformSetup).to.be.false;
-
-            office.options.viewers.Office.shouldUsePlatformSetup = true;
-            office.setup();
-            expect(office.platformSetup).to.be.true;
-        });
-
-        it('should use the platform setup if no option is passed in', () => {
-            office.options.viewers = {};
-            office.setup();
-            expect(office.platformSetup).to.be.true;
-        });
-
-        it('should still use the platform setup if the viewer option, but no setup option exists', () => {
-            office.options.viewers = {
-                Office: {
-                    disabled: false
-                }
-            };
-            office.setup();
-            expect(office.platformSetup).to.be.true;
         });
     });