fix(saving): last change is always saved now (#1170)

* fix(saving): focus on current element blurred when saving / exporting

* docs(contrib): corrected bug report template

* refactor(saving): improved conciseness

* fix(save): add contentUpdate and blurActiveElement actions

* test(save-action): add save-action tests

Co-authored-by: JPSchellenberg <jps@Lumi.education>

.github/ISSUE_TEMPLATE/bug_report.md

@@ -12,8 +12,7 @@ A clear and concise description of what the bug is.
 
 **Environment (please complete the following information):**
  - OS: [e.g. macOS]
- - Browser [e.g. chrome, safari]
- - Nodejs-Version [e.g. 22]
+ - Lumi Version
 
 **To Reproduce**
 Steps to reproduce the behavior:

client/package.json

@@ -64,6 +64,7 @@
         "@types/react-redux": "7.1.16",
         "@types/react-redux-i18n": "0.0.11",
         "@types/react-router-dom": "5.1.6",
+        "@types/redux-mock-store": "1.0.2",
         "@types/shortid": "0.0.29",
         "@types/socket.io-client": "1.4.35",
         "@types/superagent": "4.1.10",

client/src/state/H5PEditor/H5PEditorActions.ts

@@ -14,6 +14,8 @@ import {
     H5PEDITOR_SAVED,
     H5PPLAYER_INITIALIZED,
     H5PEDITOR_ERROR,
+    H5PEDITOR_BLURACTIVEELEMENT,
+    IBlurActiveElementAction,
     DeleteActions,
     LoadPlayerContentActions,
     LoadEditorContentActions,
@@ -39,7 +41,8 @@ import {
     H5PEDITOR_EXPORT_SUCCESS,
     H5PEDITOR_EXPORT_ERROR,
     H5PEDITOR_EXPORT_CANCEL,
-    H5PEDITOR_SAVE_CANCEL
+    H5PEDITOR_SAVE_CANCEL,
+    H5PEDITOR_UPDATE_CONTENT_SERVER
 } from './H5PEditorTypes';
 
 import * as selectors from './H5PEditorSelectors';
@@ -53,6 +56,11 @@ const log = new Logger('actions:tabs');
 
 declare var window: {
     h5peditor: any;
+    document: {
+        activeElement: {
+            blur: () => void;
+        };
+    };
 };
 
 export function editorLoaded(tabId: string): any {
@@ -90,9 +98,18 @@ export interface IEditorLoadedAction {
     type: typeof H5PEDITOR_LOADED;
 }
 
+export function blurActiveElement(): IBlurActiveElementAction {
+    window.document.activeElement?.blur();
+    return {
+        type: H5PEDITOR_BLURACTIVEELEMENT
+    };
+}
+
 export function exportH5P(): any {
     return async (dispatch: any) => {
         try {
+            await dispatch(updateContentOnServer());
+
             const data = await window.h5peditor.current?.save(); // this dispatches updateContent()
             dispatch({
                 payload: { contentId: data.contentId },
@@ -191,6 +208,25 @@ export function updateTab(
     };
 }
 
+export function updateContentOnServer(): any {
+    return async (dispatch: any) => {
+        // We remove the focus from the current element to make H5P save all
+        // changes in text fields
+        dispatch(blurActiveElement());
+
+        dispatch({
+            type: H5PEDITOR_UPDATE_CONTENT_SERVER
+        });
+
+        try {
+            const data = await window.h5peditor.current?.save();
+            return data;
+        } catch (error) {
+            return;
+        }
+    };
+}
+
 export function updateContent(
     tabId: string,
     contentId: string,
@@ -334,21 +370,22 @@ export function deleteH5P(
 }
 
 export function save(
-    contentId: ContentId,
     path?: string
 ): ThunkAction<void, null, null, SaveActions> {
     return async (dispatch: any) => {
-        dispatch({
-            payload: { id: contentId, path },
-            type: H5PEDITOR_SAVE_REQUEST
-        });
-
         try {
-            const response = await api.exportH5P(contentId, path);
+            const data = await dispatch(updateContentOnServer());
+
+            dispatch({
+                payload: { id: data.contentId, path },
+                type: H5PEDITOR_SAVE_REQUEST
+            });
+
+            const response = await api.exportH5P(data.contentId, path);
 
             dispatch({
                 // tslint:disable-next-line: object-shorthand-properties-first
-                payload: { id: contentId, ...response.body },
+                payload: { id: data.contentId, ...response.body },
                 type: H5PEDITOR_SAVE_SUCCESS
             });
         } catch (error) {
@@ -360,7 +397,7 @@ export function save(
             } else {
                 dispatch({
                     error,
-                    payload: { id: contentId, path },
+                    payload: { path },
                     type: H5PEDITOR_SAVE_ERROR
                 });
             }

client/src/state/H5PEditor/H5PEditorTypes.ts

@@ -20,6 +20,8 @@ export interface IH5P {
     parameters: any;
 }
 
+export const H5PEDITOR_BLURACTIVEELEMENT = 'H5P_BLURACTIVELEMENT';
+
 export const H5PEDITOR_OPEN_TAB = 'H5PEDITOR_OPEN_TAB';
 export const H5PEDITOR_CLOSE_TAB = 'H5PEDITOR_CLOSE_TAB';
 export const H5PEDITOR_SELECT_TAB = 'H5PEDITOR_SELECT_TAB';
@@ -29,6 +31,9 @@ export const H5PEDITOR_LOADED = 'H5PEDITOR_LOADED';
 export const H5PEDITOR_SAVED = 'H5PEDITOR_SAVED';
 export const H5PPLAYER_INITIALIZED = 'H5PPLAYER_INITIALIZED';
 
+export const H5PEDITOR_UPDATE_CONTENT_SERVER =
+    'H5PEDITOR_UPDATE_CONTENT_SERVER';
+
 export enum Modes {
     view,
     edit
@@ -54,6 +59,14 @@ export interface IState {
     h5peditor: IH5PEditorState;
 }
 
+export interface IUpdateContentOnServerAction {
+    type: typeof H5PEDITOR_UPDATE_CONTENT_SERVER;
+}
+
+export interface IBlurActiveElementAction {
+    type: typeof H5PEDITOR_BLURACTIVEELEMENT;
+}
+
 export interface IOpenTabAction {
     payload: {
         id: string;
@@ -218,6 +231,7 @@ export interface IH5PUpdateContentErrorAction {
 }
 
 export type UpdateContentActions =
+    | IBlurActiveElementAction
     | IH5PUpdateContentRequestAction
     | IH5PUpdateContentSuccessAction
     | IH5PUpdateContentErrorAction;

client/src/state/H5PEditor/__tests__/H5PEditorActions.test.ts

@@ -0,0 +1,70 @@
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import * as actions from '../H5PEditorActions';
+
+import {
+    H5PEDITOR_BLURACTIVEELEMENT,
+    H5PEDITOR_UPDATE_CONTENT_SERVER
+} from '../H5PEditorTypes';
+
+const middlewares = [thunk];
+const mockStore = configureMockStore(middlewares);
+
+// declare var window: any;
+
+describe('blurActiveElement', () => {
+    it('calls the window.document.activeElement?.blur() function', (done) => {
+        const store = mockStore();
+        // window = {};
+        // window.document = {};
+        // window.document.activeElement = {};
+
+        // const mockBlur = jest.fn();
+        // window.document.activeElement.blur = mockBlur;
+
+        store.dispatch(actions.blurActiveElement());
+
+        expect(store.getActions()).toEqual([
+            {
+                type: H5PEDITOR_BLURACTIVEELEMENT
+            }
+        ]);
+
+        // expect(mockBlur.mock.calls.length).toBe(1);
+
+        done();
+    });
+});
+
+describe('updateConentOnServer', () => {
+    it('blurs the active element first', (done) => {
+        const store = mockStore();
+
+        store.dispatch(actions.updateContentOnServer());
+
+        expect(store.getActions()[0]).toEqual({
+            type: H5PEDITOR_BLURACTIVEELEMENT
+        });
+        done();
+    });
+});
+
+describe('save', () => {
+    const store = mockStore();
+
+    store.dispatch(actions.save() as any);
+
+    it('blurs the active element first', (done) => {
+        expect(store.getActions()[0]).toEqual({
+            type: H5PEDITOR_BLURACTIVEELEMENT
+        });
+        done();
+    });
+
+    it('updates the content on the server', (done) => {
+        expect(store.getActions()[1]).toEqual({
+            type: H5PEDITOR_UPDATE_CONTENT_SERVER
+        });
+        done();
+    });
+});

client/src/state/index.ts

@@ -62,6 +62,6 @@ export const selectors = {
 };
 
 syncTranslationWithStore(store);
-store.dispatch(loadTranslations(translations));
-store.dispatch(setLocale('en'));
+store.dispatch(loadTranslations(translations) as any);
+store.dispatch(setLocale('en') as any);
 export default store;

client/src/views/Websocket.tsx

@@ -7,8 +7,6 @@ import Logger from '../helpers/Logger';
 import { ITab } from '../state/H5PEditor/H5PEditorTypes';
 import { actions, IState, selectors } from '../state';
 
-declare var window: any;
-
 const log = new Logger('container:websocket');
 
 interface IPassedProps {}
@@ -96,14 +94,8 @@ export class WebsocketContainer extends React.Component<
     private async saveAs(): Promise<void> {
         try {
             const { activeTab, dispatch } = this.props;
-
             log.info(`saving ${activeTab.contentId}`);
-
-            const data = await window.h5peditor.current?.save();
-
-            if (data) {
-                dispatch(actions.h5peditor.save(data.contentId));
-            }
+            dispatch(actions.h5peditor.save());
         } catch (error) {
             log.error(error);
         }
@@ -112,15 +104,8 @@ export class WebsocketContainer extends React.Component<
     private async updateAndSave(): Promise<void> {
         try {
             const { activeTab, dispatch } = this.props;
-
             log.info(`saving ${activeTab.contentId}`);
-            const data = await window.h5peditor.current?.save();
-
-            if (data) {
-                dispatch(
-                    actions.h5peditor.save(data.contentId, activeTab.path)
-                );
-            }
+            dispatch(actions.h5peditor.save(activeTab.path));
         } catch (error) {
             log.error(error);
         }

jest.config.js

@@ -103,7 +103,7 @@ module.exports = {
     // rootDir: null,
 
     // A list of paths to directories that Jest should use to search for files in
-    roots: ['<rootDir>/client', '<rootDir>/server'],
+    roots: ['<rootDir>/server'],
 
     // Allows you to use a custom runner instead of Jest's default test runner
     // runner: "jest-runner",
@@ -133,7 +133,11 @@ module.exports = {
     // ],
 
     // An array of regexp pattern strings that are matched against all test paths, matched tests are skipped
-    testPathIgnorePatterns: ['/node_modules/', '<rootDir>/test'],
+    testPathIgnorePatterns: [
+        '/node_modules/',
+        '<rootDir>/test',
+        '<rootDir>/client'
+    ],
 
     // The regexp pattern or array of patterns that Jest uses to detect test files
     // testRegex: [],

package.json

@@ -43,7 +43,7 @@
         "build:linux:dev": "CSC_IDENTITY_AUTO_DISCOVERY=false npm run build:linux",
         "publish:linux": "npx --no-install electron-builder --config builder.config.js --linux --publish always",
         "start": "NODE_ENV=development DEBUG=lumi:* PORT=8080 ./node_modules/.bin/electron .",
-        "test": "jest",
+        "test": "npm run test --prefix client/",
         "test:watch": "jest --watch",
         "test:e2e": "npx jest --config jest.e2e.config.js",
         "semantic-release": "semantic-release",