From 8a5996e3d376867c2f48f0c2af1dd1b2b5dc00d1 Mon Sep 17 00:00:00 2001 From: Mikhael Milikhin Date: Mon, 25 May 2020 13:37:57 +0700 Subject: [PATCH] Hotfixes and UI improvements for ubports-seabass@0.1.0 (#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 | 36 ++++++++-- editor/src/errors.js | 6 ++ editor/src/tabs-controller.js | 4 +- editor/tests/api-tabs.test.js | 67 ++++++++++++------- editor/tests/mocks/editor-factory.js | 1 - package.json | 2 +- ubports-seabass/html/index.html | 2 +- ubports-seabass/manifest.json.in | 2 +- ubports-seabass/po/seabass2.mikhael.pot | 2 +- ubports-seabass/qml/Main.qml | 42 ++++++++++-- .../qml/components/ErrorDialog.qml | 29 ++++++++ ubports-seabass/qml/components/FileList.qml | 3 +- ubports-seabass/qml/generic/EditorApi.qml | 8 ++- 13 files changed, 160 insertions(+), 44 deletions(-) create mode 100644 ubports-seabass/qml/components/ErrorDialog.qml diff --git a/editor/src/api.js b/editor/src/api.js index c5071239..419cc171 100644 --- a/editor/src/api.js +++ b/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) } } diff --git a/editor/src/errors.js b/editor/src/errors.js index fcef316f..7faae193 100644 --- a/editor/src/errors.js +++ b/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) + } +} diff --git a/editor/src/tabs-controller.js b/editor/src/tabs-controller.js index 3252ea85..61ccd4d5 100644 --- a/editor/src/tabs-controller.js +++ b/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) } diff --git a/editor/tests/api-tabs.test.js b/editor/tests/api-tabs.test.js index fbae4fbf..34c0d140 100644 --- a/editor/tests/api-tabs.test.js +++ b/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() }) }) diff --git a/editor/tests/mocks/editor-factory.js b/editor/tests/mocks/editor-factory.js index 43418dc9..800e8720 100644 --- a/editor/tests/mocks/editor-factory.js +++ b/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(), diff --git a/package.json b/package.json index 26463688..d16c600e 100644 --- a/package.json +++ b/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": { diff --git a/ubports-seabass/html/index.html b/ubports-seabass/html/index.html index 56215762..a8d46efa 100644 --- a/ubports-seabass/html/index.html +++ b/ubports-seabass/html/index.html @@ -49,7 +49,7 @@

- Seabass for UBports v0.1.0 + Seabass for UBports v0.1.1

Release notes diff --git a/ubports-seabass/manifest.json.in b/ubports-seabass/manifest.json.in index d4a32a8c..ae6c7ee0 100644 --- a/ubports-seabass/manifest.json.in +++ b/ubports-seabass/manifest.json.in @@ -9,7 +9,7 @@ "desktop": "seabass2.desktop" } }, - "version": "0.1.0", + "version": "0.1.1", "maintainer": "Mikhael Milikhin ", "framework" : "ubuntu-sdk-16.04" } diff --git a/ubports-seabass/po/seabass2.mikhael.pot b/ubports-seabass/po/seabass2.mikhael.pot index b690b7ab..91f1bcef 100644 --- a/ubports-seabass/po/seabass2.mikhael.pot +++ b/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 \n" "Language-Team: LANGUAGE \n" diff --git a/ubports-seabass/qml/Main.qml b/ubports-seabass/qml/Main.qml index f5d387d1..83effaad 100644 --- a/ubports-seabass/qml/Main.qml +++ b/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 }) diff --git a/ubports-seabass/qml/components/ErrorDialog.qml b/ubports-seabass/qml/components/ErrorDialog.qml new file mode 100644 index 00000000..92c7b742 --- /dev/null +++ b/ubports-seabass/qml/components/ErrorDialog.qml @@ -0,0 +1,29 @@ +import QtQuick 2.4 +import Ubuntu.Components 1.3 +import Ubuntu.Components.Popups 1.3 +import "../generic/utils.js" as QmlJs + +Item { + property string message: qsTr('unknown error') + + function show(errorMsg) { + message = errorMsg + PopupUtils.open(dialog) + } + + Component { + id: dialog + + Dialog { + id: dialogue + title: qsTr('Error occured') + text: message + Button { + text: qsTr("Close") + onClicked: { + PopupUtils.close(dialogue) + } + } + } + } +} diff --git a/ubports-seabass/qml/components/FileList.qml b/ubports-seabass/qml/components/FileList.qml index be609b74..22731272 100644 --- a/ubports-seabass/qml/components/FileList.qml +++ b/ubports-seabass/qml/components/FileList.qml @@ -10,6 +10,7 @@ ListView { property bool isPage: false property bool showHidden: false property string homeDir + property real rowHeight: units.gu(4.5) readonly property string backgroundColor: theme.palette.normal.background readonly property string textColor: theme.palette.normal.backgroundText @@ -42,7 +43,7 @@ ListView { model: folderModel delegate: ListItem { visible: isVisible() - height: isVisible() ? undefined : 0 + height: isVisible() ? rowHeight : 0 color: backgroundColor MouseArea { diff --git a/ubports-seabass/qml/generic/EditorApi.qml b/ubports-seabass/qml/generic/EditorApi.qml index 79faf844..c1cb937a 100644 --- a/ubports-seabass/qml/generic/EditorApi.qml +++ b/ubports-seabass/qml/generic/EditorApi.qml @@ -76,18 +76,22 @@ QtObject { * Saves file with the given content at the given path * @param {string} filePath - /path/to/file * @param {string} content - file content + * @param {function} [callback] - callback * @returns {undefined} */ - function saveFile(filePath, content) { + function saveFile(filePath, content, callback) { + callback = callback || function emptyCallback() {} isSaveInProgress = true return QmlJs.writeFile(filePath, content, function(err) { isSaveInProgress = false if (err) { console.error(err) - return errorOccured(qsTr('Unable to write the file. Please ensure that you have write access to') + ' ' + filePath) + errorOccured(qsTr('Unable to write the file. Please ensure that you have write access to') + ' ' + filePath) + return callback(err) } postMessage('fileSaved', { filePath: filePath, content: content }) + return callback(null) }) }