feat(popover): sort keymaps and macros alphabetically (#534)

* feat(popover): sort keymaps and macros alphabetically

Closes #512

* small performance refactoring

packages/uhk-web/src/app/components/side-menu/side-menu.component.ts

@@ -56,17 +56,8 @@ export class SideMenuComponent implements AfterContentInit, OnDestroy {
             addon: 'active'
         };
 
-        this.keymaps$ = store.let(getKeymaps())
+        this.keymaps$ = store.let(getKeymaps());
-            .map(keymaps => keymaps.slice()) // Creating a new array reference, because the sort is working in place
+        this.macros$ = store.let(getMacros());
-            .do((keymaps: Keymap[]) => {
-                keymaps.sort((first: Keymap, second: Keymap) => first.name.localeCompare(second.name));
-            });
-
-        this.macros$ = store.let(getMacros())
-            .map(macros => macros.slice()) // Creating a new array reference, because the sort is working in place
-            .do((macros: Macro[]) => {
-                macros.sort((first: Macro, second: Macro) => first.name.localeCompare(second.name));
-            });
 
         this.showAddonMenu$ = this.store.select(showAddonMenu);
         this.runInElectron$ = this.store.select(runningInElectron);

packages/uhk-web/src/app/store/effects/keymap.ts

@@ -7,14 +7,17 @@ import { Observable } from 'rxjs/Observable';
 
 import 'rxjs/add/operator/do';
 import 'rxjs/add/operator/map';
+import 'rxjs/add/operator/pairwise';
 import 'rxjs/add/operator/startWith';
 import 'rxjs/add/operator/switchMap';
 import 'rxjs/add/operator/withLatestFrom';
 import 'rxjs/add/observable/of';
 
 import { Keymap } from 'uhk-common';
+import { findNewItem } from '../../util';
 import { KeymapActions } from '../actions';
 import { AppState } from '../index';
+import { getKeymaps } from '../reducers/user-configuration';
 
 @Injectable()
 export class KeymapEffects {
@@ -32,10 +35,10 @@ export class KeymapEffects {
 
     @Effect({ dispatch: false }) addOrDuplicate$: any = this.actions$
         .ofType(KeymapActions.ADD, KeymapActions.DUPLICATE)
-        .withLatestFrom(this.store)
+        .withLatestFrom(this.store.let(getKeymaps()).pairwise(), (action, latest) => latest)
-        .map(latest => latest[1].userConfiguration.keymaps)
+        .do(([prevKeymaps, newKeymaps]) => {
-        .do(keymaps => {
+            const newKeymap = findNewItem(prevKeymaps, newKeymaps);
-            this.router.navigate(['/keymap', keymaps[keymaps.length - 1].abbreviation]);
+            this.router.navigate(['/keymap', newKeymap.abbreviation]);
         });
 
     @Effect({ dispatch: false }) remove$: any = this.actions$

packages/uhk-web/src/app/store/effects/macro.ts

@@ -2,14 +2,18 @@ import { Injectable } from '@angular/core';
 import { Router } from '@angular/router';
 
 import { Actions, Effect } from '@ngrx/effects';
-import { Store } from '@ngrx/store';
+import { Store, Action } from '@ngrx/store';
 
 import 'rxjs/add/operator/do';
 import 'rxjs/add/operator/map';
+import 'rxjs/add/operator/pairwise';
 import 'rxjs/add/operator/withLatestFrom';
 
+import { Macro } from 'uhk-common';
 import { KeymapActions, MacroActions } from '../actions';
 import { AppState } from '../index';
+import { getMacros } from '../reducers/user-configuration';
+import { findNewItem } from '../../util';
 
 @Injectable()
 export class MacroEffects {
@@ -27,22 +31,16 @@ export class MacroEffects {
             }
         });
 
-    @Effect({ dispatch: false }) add$: any = this.actions$
+    @Effect({ dispatch: false }) addOrDuplicate$: any = this.actions$
-        .ofType(MacroActions.ADD)
+        .ofType(MacroActions.ADD, MacroActions.DUPLICATE)
-        .withLatestFrom(this.store)
+        .withLatestFrom(this.store.let(getMacros()).pairwise(), (action, latest) => ([action, latest[0], latest[1]]))
-        .map(([action, state]) => state.userConfiguration.macros)
+        .do(([action, prevMacros, newMacros]: [Action, Macro[], Macro[]]) => {
-        .map(macros => macros[macros.length - 1])
+            const newMacro = findNewItem(prevMacros, newMacros);
-        .do(lastMacro => {
+            const commands = ['/macro', newMacro.id];
-            this.router.navigate(['/macro', lastMacro.id, 'new']);
+            if (action.type === MacroActions.ADD) {
-        });
+                commands.push('new');
-
+            }
-    @Effect({ dispatch: false }) duplicate: any = this.actions$
+            this.router.navigate(commands);
-        .ofType(MacroActions.DUPLICATE)
-        .withLatestFrom(this.store)
-        .map(([action, state]) => state.userConfiguration.macros)
-        .map(macros => macros[macros.length - 1])
-        .do(lastMacro => {
-            this.router.navigate(['/macro', lastMacro.id]);
         });
 
     constructor(private actions$: Actions, private router: Router, private store: Store<AppState>) { }

packages/uhk-web/src/app/store/reducers/user-configuration.ts

@@ -19,7 +19,12 @@ export function reducer(state = initialState, action: Action & { payload?: any }
         case ActionTypes.APPLY_USER_CONFIGURATION_FROM_FILE:
         case ActionTypes.LOAD_RESET_USER_CONFIGURATION:
         case ActionTypes.LOAD_USER_CONFIG_SUCCESS: {
-            return Object.assign(changedUserConfiguration, action.payload);
+            Object.assign(changedUserConfiguration, action.payload);
+            changedUserConfiguration.keymaps = [...changedUserConfiguration.keymaps];
+            changedUserConfiguration.keymaps.sort((first: Keymap, second: Keymap) => first.name.localeCompare(second.name));
+            changedUserConfiguration.macros = [...changedUserConfiguration.macros];
+            changedUserConfiguration.macros.sort((first: Macro, second: Macro) => first.name.localeCompare(second.name));
+            return changedUserConfiguration;
         }
 
         case KeymapActions.ADD:
@@ -29,7 +34,7 @@ export function reducer(state = initialState, action: Action & { payload?: any }
             newKeymap.name = generateName(state.keymaps, newKeymap.name);
             newKeymap.isDefault = (state.keymaps.length === 0);
 
-            changedUserConfiguration.keymaps = state.keymaps.concat(newKeymap);
+            changedUserConfiguration.keymaps = insertItemInNameOrder(state.keymaps, newKeymap);
             break;
         }
         case KeymapActions.EDIT_NAME: {
@@ -38,20 +43,27 @@ export function reducer(state = initialState, action: Action & { payload?: any }
             }
 
             const name: string = action.payload.name.trim();
+            let keymapToRename: Keymap = null;
 
             const duplicate = state.keymaps.some((keymap: Keymap) => {
+                if (keymap.abbreviation === action.payload.abbr) {
+                    keymapToRename = keymap;
+                }
+
                 return keymap.name === name && keymap.abbreviation !== action.payload.abbr;
             });
 
-            changedUserConfiguration.keymaps = state.keymaps.map((keymap: Keymap) => {
+            if (duplicate) {
-                keymap = Object.assign(new Keymap(), keymap);
+                break;
-
-                if (!duplicate && keymap.abbreviation === action.payload.abbr) {
-                    keymap.name = name;
             }
-                return keymap;
-            });
 
+            const newKeymap = Object.assign(new Keymap(), keymapToRename, { name });
+
+            changedUserConfiguration.keymaps = insertItemInNameOrder(
+                state.keymaps,
+                newKeymap,
+                keymap => keymap.abbreviation !== newKeymap.abbreviation
+            );
             break;
         }
         case KeymapActions.EDIT_ABBR: {
@@ -163,7 +175,7 @@ export function reducer(state = initialState, action: Action & { payload?: any }
             newMacro.isPrivate = true;
             newMacro.macroActions = [];
 
-            changedUserConfiguration.macros = state.macros.concat(newMacro);
+            changedUserConfiguration.macros = insertItemInNameOrder(state.macros, newMacro);
             break;
         }
         case MacroActions.DUPLICATE: {
@@ -171,7 +183,7 @@ export function reducer(state = initialState, action: Action & { payload?: any }
             newMacro.name = generateName(state.macros, newMacro.name);
             newMacro.id = generateMacroId(state.macros);
 
-            changedUserConfiguration.macros = state.macros.concat(newMacro);
+            changedUserConfiguration.macros = insertItemInNameOrder(state.macros, newMacro);
             break;
         }
         case MacroActions.EDIT_NAME: {
@@ -180,20 +192,22 @@ export function reducer(state = initialState, action: Action & { payload?: any }
             }
 
             const name: string = action.payload.name.trim();
+            let macroToRename: Macro = null;
 
             const duplicate = state.macros.some((macro: Macro) => {
+                if (macro.id === action.payload.id) {
+                   macroToRename = macro;
+                }
+
                 return macro.id !== action.payload.id && macro.name === name;
             });
 
-            changedUserConfiguration.macros = state.macros.map((macro: Macro) => {
+            if (duplicate) {
-                macro = Object.assign(new Macro(), macro);
+                break;
-                if (!duplicate && macro.id === action.payload.id) {
-                    macro.name = name;
             }
 
-                return macro;
+            const newMacro = Object.assign(new Macro(), macroToRename, { name });
-            });
+            changedUserConfiguration.macros = insertItemInNameOrder(state.macros, newMacro, macro => macro.id !== newMacro.id);
-
             break;
         }
         case MacroActions.REMOVE:
@@ -372,6 +386,27 @@ function generateMacroId(macros: Macro[]) {
     return newId + 1;
 }
 
+function insertItemInNameOrder<T extends { name: string }>(
+    items: T[], newItem: T, keepItem: (item: T) => boolean = () => true
+): T[] {
+    const newItems: T[] = [];
+    let added = false;
+    for (const item of items) {
+        if (!added && item.name.localeCompare(newItem.name) > 0) {
+            newItems.push(newItem);
+            added = true;
+        }
+        if (keepItem(item)) {
+            newItems.push(item);
+        }
+    }
+    if (!added) {
+        newItems.push(newItem);
+    }
+
+    return newItems;
+}
+
 function checkExistence(layers: Layer[], property: string, value: any): Layer[] {
     const keyActionsToClear: {
         layerIdx: number,

packages/uhk-web/src/app/util/find-new-item.ts

@@ -0,0 +1,9 @@
+export function findNewItem<T>(oldItems: T[], newItems: T[]): T {
+    for (let i = 0; i < oldItems.length; ++i) {
+        if (oldItems[i] !== newItems[i]) {
+            return newItems[i];
+        }
+    }
+
+    return newItems[newItems.length - 1];
+}

packages/uhk-web/src/app/util/index.ts

@@ -1,3 +1,4 @@
+export * from './find-new-item';
 export * from './html-helper';
 export * from './validators';
 export * from './version-helper';