Hotfixes and UI improvements for [email protected] (#14)

* [ubports-seabass] fix issues with saving large files

* [ubports-seabass] add Error dialog, use theme background color as page background, add 'Ctrl+S' shortcut, [editor] separate NotFound and InvalidArg errors

* [editor] update unit tests to math current API

editor/src/api.js

@@ -1,5 +1,5 @@
 /* globals localStorage */
-import { InvalidArgError } from './errors'
+import { InvalidArgError, NotFoundError } from './errors'
 import TabsController from './tabs-controller'
 
 class Api {
@@ -52,6 +52,16 @@ class Api {
   //   this._tabsController.exec(filePath, 'beautify')
   // }
 
+  /**
+   * Returns file content.
+   *  API backend must support returning results from JS calls to use the method
+   * @param {string} filePath - path/to/file
+   * @returns {string} - file content
+   */
+  _apiOnGetFileContent ({ filePath }) {
+    return this._tabsController.exec(filePath, 'getContent')
+  }
+
   /**
    * 'navigateLeft' command handler: intended to move cursor left
    * @param {string} filePath - /path/to/file
@@ -171,12 +181,21 @@ class Api {
    * @returns {undefined}
    */
   _apiOnRequestSaveFile ({ filePath }) {
-    const value = this._tabsController.exec(filePath, 'getContent')
-    this._sendApiMessage('saveFile', {
-      content: value,
-      filePath,
-      responseTo: 'requestSaveFile'
-    })
+    try {
+      const value = this._tabsController.exec(filePath, 'getContent')
+      this._sendApiMessage('saveFile', {
+        content: value,
+        filePath,
+        responseTo: 'requestSaveFile'
+      })
+    } catch (err) {
+      if (err instanceof NotFoundError) {
+        // requestSaveFile operation must throw error to the platform application if required file is not loaded
+        throw new InvalidArgError(`File ${filePath} is not opened`)
+      }
+
+      throw err
+    }
   }
 
   /**
@@ -239,6 +258,9 @@ class Api {
       }
       return this[apiMethod](data)
     } catch (err) {
+      if (err instanceof NotFoundError) {
+        return console.warn(err)
+      }
       this._sendApiError(err.message)
     }
   }

editor/src/errors.js

@@ -12,3 +12,9 @@ export class InvalidArgError extends BaseError {
     super('InvalidArgError', message)
   }
 }
+
+export class NotFoundError extends BaseError {
+  constructor (message) {
+    super('NotFoundError', message)
+  }
+}

editor/src/tabs-controller.js

@@ -1,4 +1,4 @@
-import { InvalidArgError } from './errors'
+import { NotFoundError } from './errors'
 
 export default class TabsController {
   constructor ({ rootElem, editorFactory, onStateChange }) {
@@ -61,7 +61,7 @@ export default class TabsController {
   exec (filePath, action, ...args) {
     const tab = this._getTab(filePath)
     if (!tab) {
-      throw new InvalidArgError(`file ${filePath} is not loaded`)
+      throw new NotFoundError(`file ${filePath} is not loaded`)
     }
     return tab.editor[action](...args)
   }

editor/tests/api-tabs.test.js

@@ -1,7 +1,8 @@
-/* globals describe,expect,jest,it,beforeEach,beforeAll,afterAll,localStorage */
+/* globals describe,expect,jest,it,beforeEach,beforeAll,afterAll,afterEach,localStorage */
 
-import registerApi from '../src/api' //eslint-disable-line
-import uuid from 'uuid/v4'//eslint-disable-line
+import registerApi from '../src/api'
+import { NotFoundError } from '../src/errors'
+import { v4 as uuid } from 'uuid'
 import editorFactory from './mocks/editor-factory'
 
 describe('editor API', () => {
@@ -14,12 +15,11 @@ describe('editor API', () => {
     navigator.qt.onmessage({ data: JSON.stringify(payload) })
   }
 
-  const createEditor = () => {
-    const api = registerApi({ editorFactory, rootElem, welcomeElem })
+  const createEditor = (apiBackend = 'navigatorQt') => {
+    const api = registerApi({ editorFactory, apiBackend, rootElem, welcomeElem })
     api._tabsController.create(filePath)
     const editor = api._tabsController._tabs[0].editor
     editor.getContent.mockReturnValue(content)
-    editor.getFilePath.mockReturnValue(uuid())
     return { api, editor }
   }
 
@@ -60,7 +60,6 @@ describe('editor API', () => {
   describe('#fileSaved', () => {
     it('should execute `setSavedContent` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'fileSaved',
@@ -156,7 +155,6 @@ describe('editor API', () => {
     it('should send API message with file content', () => {
       const { editor } = createEditor()
       editor.getContent.mockReturnValue(content)
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'requestSaveFile',
@@ -195,7 +193,6 @@ describe('editor API', () => {
   describe('#undo', () => {
     it('should execute `undo` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'undo',
@@ -209,7 +206,6 @@ describe('editor API', () => {
   describe('#redo', () => {
     it('should execute `redo` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'redo',
@@ -223,7 +219,6 @@ describe('editor API', () => {
   describe('#navigateLeft', () => {
     it('should execute `navigateLeft` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateLeft',
@@ -237,7 +232,6 @@ describe('editor API', () => {
   describe('#navigateRight', () => {
     it('should execute `navigateRight` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateRight',
@@ -251,7 +245,6 @@ describe('editor API', () => {
   describe('#navigateUp', () => {
     it('should execute `navigateUp` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateUp',
@@ -265,7 +258,6 @@ describe('editor API', () => {
   describe('#navigateDown', () => {
     it('should execute `navigateDown` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateDown',
@@ -279,7 +271,6 @@ describe('editor API', () => {
   describe('#navigateLineStart', () => {
     it('should execute `navigateLineStart` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateLineStart',
@@ -293,7 +284,6 @@ describe('editor API', () => {
   describe('#navigateLineEnd', () => {
     it('should execute `navigateLineEnd` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateLineEnd',
@@ -307,7 +297,6 @@ describe('editor API', () => {
   describe('#navigateFileStart', () => {
     it('should execute `navigateFileStart` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateFileStart',
@@ -321,7 +310,6 @@ describe('editor API', () => {
   describe('#navigateFileEnd', () => {
     it('should execute `navigateFileEnd` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'navigateFileEnd',
@@ -358,9 +346,16 @@ describe('editor API', () => {
   })
 
   describe('#toggleReadOnly', () => {
+    beforeEach(() => {
+      console.warn = jest.fn()
+    })
+
+    afterEach(() => {
+      console.warn.mockRestore()
+    })
+
     it('should execute `toggleReadOnly` action', () => {
       const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
 
       postMessage({
         action: 'toggleReadOnly',
@@ -369,21 +364,47 @@ describe('editor API', () => {
 
       expect(editor.toggleReadOnly).toHaveBeenCalledTimes(1)
     })
+
+    it('should not throw if filePath is incorrect', () => {
+      const { editor } = createEditor()
+      const invalidFilePath = uuid()
+
+      postMessage({
+        action: 'toggleReadOnly',
+        data: { filePath: invalidFilePath }
+      })
+
+      expect(console.warn).toBeCalledWith(expect.any(NotFoundError))
+      expect(editor.toggleReadOnly).toHaveBeenCalledTimes(0)
+    })
+  })
+
+  describe('#getFileContent', () => {
+    it('should execute `getContent` action', () => {
+      const { editor } = createEditor('url')
+      const expectedValue = uuid()
+      editor.getContent.mockReturnValue(expectedValue)
+
+      const value = window.postSeabassApiMessage({
+        action: 'getFileContent',
+        data: { filePath }
+      })
+
+      expect(value).toEqual(expectedValue)
+    })
   })
 
   describe('#unknownMethod', () => {
-    const originalConsoleWarn = console.warn
     beforeAll(() => {
       console.warn = jest.fn()
     })
 
     afterAll(() => {
-      console.warn = originalConsoleWarn
+      console.warn.mockRestore()
     })
 
     it('should not throw', () => {
-      const { editor } = createEditor()
-      editor.getFilePath.mockReturnValue(filePath)
+      createEditor()
 
       postMessage({ action: uuid() })
     })

editor/tests/mocks/editor-factory.js

@@ -4,7 +4,6 @@ export default () => ({
   activate: jest.fn(),
   destroy: jest.fn(),
   getContent: jest.fn(),
-  getFilePath: jest.fn(),
   loadFile: jest.fn(),
   onChange: jest.fn(),
   redo: jest.fn(),

package.json

@@ -1,6 +1,6 @@
 {
   "name": "seabass-editor",
-  "version": "0.3.1",
+  "version": "0.4.0",
   "description": "web-editor component for Seabass app",
   "main": "index.js",
   "scripts": {

ubports-seabass/html/index.html

@@ -49,7 +49,7 @@
 <body>
   <div id="welcome">
     <h1>
-      Seabass for UBports v0.1.0
+      Seabass for UBports v0.1.1
     </h1>
     <h2>
       Release notes

ubports-seabass/manifest.json.in

@@ -9,7 +9,7 @@
             "desktop":  "seabass2.desktop"
         }
     },
-    "version": "0.1.0",
+    "version": "0.1.1",
     "maintainer": "Mikhael Milikhin <[email protected]>",
     "framework" : "ubuntu-sdk-16.04"
 }

ubports-seabass/po/seabass2.mikhael.pot

@@ -8,7 +8,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: seabass2.mikhael\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2020-05-21 15:45+0000\n"
+"POT-Creation-Date: 2020-05-25 06:32+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[email protected]>\n"

ubports-seabass/qml/Main.qml

@@ -33,6 +33,9 @@ MainView {
   GenericComponents.EditorApi {
     id: api
     homeDir: StandardPaths.writableLocation(StandardPaths.HomeLocation)
+    onErrorOccured: function(message) {
+      errorDialog.show(message)
+    }
     onMessageSent: function(jsonPayload) {
       editor.runJavaScript("window.postSeabassApiMessage(" + jsonPayload + ")");
     }
@@ -45,6 +48,26 @@ MainView {
       file.hasChanges = hasChanges
       filesModel.set(fileIndex, file)
     }
+
+    /**
+     * Returns current content of the given file from the EditorApi
+     *  (the API backend must support returning a result from a JS call for this method to work)
+     * @param {function} - callback
+     * @returns {string} - file content
+     */
+    function getFileContent(callback) {
+      const jsonPayload = JSON.stringify({
+        action: 'getFileContent',
+        data: {
+          filePath: filePath
+        }
+      })
+      return editor.runJavaScript("window.postSeabassApiMessage(" + jsonPayload + ")", callback);
+    }
+  }
+
+  CustomComponents.ErrorDialog {
+    id: errorDialog
   }
 
   CustomComponents.SaveDialog {
@@ -54,6 +77,7 @@ MainView {
   Page {
     id: page
     anchors.fill: parent
+    background: theme.palette.normal.background
 
     RowLayout {
       anchors.fill: parent
@@ -124,7 +148,12 @@ MainView {
                 iconName: "save"
                 text: qsTr("Save")
                 enabled: api.filePath && api.hasChanges
-                onTriggered: api.requestSaveFile()
+                shortcut: StandardKey.Save
+                onTriggered: {
+                  api.getFileContent(function(fileContent) {
+                    api.saveFile(api.filePath, fileContent)
+                  })
+                }
               }
             ]
           }
@@ -153,9 +182,14 @@ MainView {
 
             saveDialog.show(file.filePath, {
               onSaved: function() {
-                api.requestSaveFile()
-                // TODO: wait until the file actually saved
-                __close()
+                const filePath = api.filePath
+                api.getFileContent(function(fileContent) {
+                  api.saveFile(filePath, fileContent, function(err) {
+                    if (!err) {
+                      __close()
+                    }
+                  })
+                })
               },
               onDismissed: __close
             })