Cross Oni Movement (#1747)

* First messy attempt at moving around Oni windows.

* Make slightly smarter.

* Tidy up.

* Fixed passing of direction.

Also check that there are valid windows to swap to, both initially and after filtering.

* Tidy up by moving to new file.

* Ignore minimized windows.

Not sure if this should be the default or not.
Is possible we could hook up a config option and pass that over the IPC to here to make it configurable.

* Add to OniEditor for now.

* Remove temp commands.

* Fix lint issues.

* Remove main window from main.ts

Added function to get current active window, and renamed main window to currentWindow.

* Extend the logic of window swapping to check the main axis first.

I.e. always move to the closest window on the right.

* Fix lint issues.

* Setup main unit tests.

* Change code to make it more easily unit-testable.

* Add first set of tests.

* Add further tests.

* Tidy up tests.

* Remove from OniEditor.

* Wire up in index.tsx.

browser/src/Services/MultiProcess.ts

@@ -5,6 +5,7 @@
  */
 
 import { ipcRenderer } from "electron"
+import { WindowManager } from "./WindowManager"
 
 export class MultiProcess {
     public focusPreviousInstance(): void {
@@ -14,6 +15,18 @@ export class MultiProcess {
     public focusNextInstance(): void {
         ipcRenderer.send("focus-next-instance")
     }
+
+    public moveToNextOniInstance(direction: string): void {
+        ipcRenderer.send("move-to-next-oni-instance", direction)
+    }
+}
+
+export const activate = (windowManager: WindowManager): void => {
+    // Wire up accepting unhandled moves to swap to the next
+    // available Oni instance.
+    windowManager.onUnhandledMove.subscribe((direction: string) => {
+        multiProcess.moveToNextOniInstance(direction)
+    })
 }
 
 export const multiProcess = new MultiProcess()

browser/src/index.tsx

@@ -52,6 +52,8 @@ const start = async (args: string[]): Promise<void> => {
     const terminalPromise = import("./Services/Terminal")
     const workspacePromise = import("./Services/Workspace")
     const workspaceCommandsPromise = import("./Services/Workspace/WorkspaceCommands")
+    const windowManagerPromise = import("./Services/WindowManager")
+    const multiProcessPromise = import("./Services/MultiProcess")
 
     const themePickerPromise = import("./Services/Themes/ThemePicker")
     const cssPromise = import("./CSS")
@@ -119,6 +121,11 @@ const start = async (args: string[]): Promise<void> => {
     Workspace.activate(configuration, editorManager)
     const workspace = Workspace.getInstance()
 
+    const WindowManager = await windowManagerPromise
+    const MultiProcess = await multiProcessPromise
+
+    MultiProcess.activate(WindowManager.windowManager)
+
     const StatusBar = await statusBarPromise
     StatusBar.activate(configuration)
     const statusBar = StatusBar.getInstance()

main/src/WindowManager.ts

@@ -0,0 +1,155 @@
+import { BrowserWindow, Rectangle } from "electron"
+import * as Log from "./Log"
+
+// Function to process moving to the next Oni window, given a direction.
+export function moveToNextOniInstance(windows: BrowserWindow[], direction: string) {
+    const currentFocusedWindows = windows.filter(f => f.isFocused())
+
+    if (currentFocusedWindows.length === 0) {
+        Log.info("No window currently focused")
+        return
+    } else if (windows.length === 1) {
+        Log.info("No window to swap to")
+        return
+    }
+
+    const currentFocusedWindow = currentFocusedWindows[0]
+    const windowsToCheck = windows.filter(
+        window => window !== currentFocusedWindow && !window.isMinimized(),
+    )
+
+    const validWindows = windowsToCheck.filter(window =>
+        windowIsInValidDirection(direction, currentFocusedWindow.getBounds(), window.getBounds()),
+    )
+
+    if (validWindows.length === 0) {
+        return
+    }
+
+    const windowToSwapTo = validWindows.reduce<BrowserWindow>((curr, prev) => {
+        const isCurrentWindowBetter = checkWindowToFindBest(
+            currentFocusedWindow.getBounds(),
+            curr.getBounds(),
+            prev.getBounds(),
+            direction,
+        )
+
+        if (isCurrentWindowBetter) {
+            return curr
+        } else {
+            return prev
+        }
+    }, validWindows[0])
+
+    windows[windows.indexOf(windowToSwapTo)].focus()
+}
+
+export function windowIsInValidDirection(
+    direction: string,
+    currentPos: Rectangle,
+    testPos: Rectangle,
+) {
+    let valuesIncrease = false
+    let coord = "x"
+
+    switch (direction) {
+        case "left":
+            valuesIncrease = false
+            break
+        case "right":
+            valuesIncrease = true
+            break
+        case "up":
+            valuesIncrease = false
+            coord = "y"
+            break
+        case "down":
+            valuesIncrease = true
+            coord = "y"
+            break
+        default:
+            return false
+    }
+
+    // Check if the screen we are testing is in the right direction.
+    // shouldBeBigger is used for moving to the right or down, since the X/Y values increase.
+    // Othewise, we want the value that decreases (i.e. for left or up)
+    if (valuesIncrease) {
+        if (testPos[coord] > currentPos[coord]) {
+            return true
+        }
+    } else {
+        if (testPos[coord] < currentPos[coord]) {
+            return true
+        }
+    }
+
+    return false
+}
+
+// Given a window, check if it is the best window seen so far.
+// This is determined by the difference in X and Y relative to the current window.
+export function checkWindowToFindBest(
+    currentWindow: Rectangle,
+    testWindow: Rectangle,
+    currentBest: Rectangle,
+    direction: string,
+) {
+    const differenceInX = Math.abs(currentWindow.x - testWindow.x)
+    const differenceInY = Math.abs(currentWindow.y - testWindow.y)
+
+    const bestDiffInX = Math.abs(currentWindow.x - currentBest.x)
+    const bestDiffInY = Math.abs(currentWindow.y - currentBest.y)
+
+    // Use the main axis such that we always move to the closest window in the
+    // direction we are moving.
+    let mainAxisDiff = DistanceComparison.larger
+    let secondAxisDiff = DistanceComparison.larger
+
+    switch (direction) {
+        case "left":
+        case "right":
+            mainAxisDiff = compareDistances(differenceInX, bestDiffInX)
+            secondAxisDiff = compareDistances(differenceInY, bestDiffInY)
+            break
+        case "up":
+        case "down":
+            mainAxisDiff = compareDistances(differenceInY, bestDiffInY)
+            secondAxisDiff = compareDistances(differenceInX, bestDiffInX)
+            break
+        default:
+            return false
+    }
+
+    // If an equal distance away, we should check the other axis and
+    // take the one that is closer in that axis.
+    // If they are both the same? Just use the current one.
+    if (mainAxisDiff === DistanceComparison.smaller) {
+        return true
+    } else if (
+        mainAxisDiff === DistanceComparison.equal &&
+        secondAxisDiff === DistanceComparison.smaller
+    ) {
+        return true
+    } else {
+        return false
+    }
+}
+
+export enum DistanceComparison {
+    smaller,
+    larger,
+    equal,
+}
+
+// Helper function to compare the distances and return how the values
+// compare.
+export function compareDistances(currentDifference: number, bestDifference: number) {
+    if (currentDifference === bestDifference) {
+        return DistanceComparison.equal
+    } else if (currentDifference < bestDifference) {
+        return DistanceComparison.smaller
+    } else {
+        return DistanceComparison.larger
+    }
+}

main/src/main.ts

@@ -9,6 +9,7 @@ import addDevExtensions from "./installDevTools"
 import * as Log from "./Log"
 import { buildDockMenu, buildMenu } from "./menu"
 import { makeSingleInstance } from "./ProcessLifecycle"
+import { moveToNextOniInstance } from "./WindowManager"
 
 global["getLogs"] = Log.getAllLogs // tslint:disable-line no-string-literal
 
@@ -75,10 +76,17 @@ ipcMain.on("focus-previous-instance", () => {
     focusNextInstance(-1)
 })
 
+ipcMain.on("move-to-next-oni-instance", (event, direction: string) => {
+    Log.info(`Attempting to swap to Oni instance on the ${direction}.`)
+    moveToNextOniInstance(windows, direction)
+})
+
 // Keep a global reference of the window object, if you don't, the window will
 // be closed automatically when the JavaScript object is garbage collected.
 let windows: BrowserWindow[] = []
-let mainWindow: BrowserWindow = null
+const activeWindow = () => {
+    return windows.filter(w => w.isFocused())[0] || null
+}
 
 // Only enable 'single-instance' mode when we're not in the hot-reload mode
 // Otherwise, all other open instances will also pick up the webpack bundle
@@ -149,7 +157,7 @@ export function createWindow(
     const indexPath = path.join(rootPath, "index.html?react_perf")
     // Create the browser window.
     // TODO: Do we need to use non-ico for other platforms?
-    mainWindow = new BrowserWindow({
+    let currentWindow = new BrowserWindow({
         icon: iconPath,
         webPreferences,
         backgroundColor,
@@ -161,12 +169,12 @@ export function createWindow(
     })
 
     if (windowState.isMaximized) {
-        mainWindow.maximize()
+        currentWindow.maximize()
     }
 
-    updateMenu(mainWindow, false)
-    mainWindow.webContents.on("did-finish-load", () => {
-        mainWindow.webContents.send("init", {
+    updateMenu(currentWindow, false)
+    currentWindow.webContents.on("did-finish-load", () => {
+        currentWindow.webContents.send("init", {
             args: commandLineArguments,
             workingDirectory,
         })
@@ -176,55 +184,55 @@ export function createWindow(
         Log.info("Oni started")
 
         if (delayedEvent) {
-            mainWindow.webContents.send(delayedEvent.evt, ...delayedEvent.cmd)
+            currentWindow.webContents.send(delayedEvent.evt, ...delayedEvent.cmd)
         }
     })
 
     ipcMain.on("rebuild-menu", (_evt, loadInit) => {
         // ipcMain is a singleton so if there are multiple Oni instances
         // we may receive an event from a different instance
-        if (mainWindow) {
-            updateMenu(mainWindow, loadInit)
+        if (currentWindow) {
+            updateMenu(currentWindow, loadInit)
         }
     })
 
     // and load the index.html of the app.
-    mainWindow.loadURL(`file://${indexPath}`)
+    currentWindow.loadURL(`file://${indexPath}`)
 
     // Open the DevTools.
     if (process.env.NODE_ENV === "development" || commandLineArguments.indexOf("--debug") >= 0) {
-        mainWindow.webContents.openDevTools()
+        currentWindow.webContents.openDevTools()
     }
 
-    mainWindow.on("move", () => {
-        storeWindowState(mainWindow)
+    currentWindow.on("move", () => {
+        storeWindowState(currentWindow)
     })
-    mainWindow.on("resize", () => {
-        storeWindowState(mainWindow)
+    currentWindow.on("resize", () => {
+        storeWindowState(currentWindow)
     })
-    mainWindow.on("close", () => {
-        storeWindowState(mainWindow)
+    currentWindow.on("close", () => {
+        storeWindowState(currentWindow)
     })
 
     // Emitted when the window is closed.
-    mainWindow.on("closed", () => {
+    currentWindow.on("closed", () => {
         // Dereference the window object, usually you would store windows
         // in an array if your app supports multi windows, this is the time
         // when you should delete the corresponding element.
-        windows = windows.filter(m => m !== mainWindow)
-        mainWindow = null
+        windows = windows.filter(m => m !== currentWindow)
+        currentWindow = null
     })
 
-    windows.push(mainWindow)
+    windows.push(currentWindow)
 
-    return mainWindow
+    return currentWindow
 }
 
 app.on("open-file", (event, filePath) => {
     event.preventDefault()
     Log.info(`filePath to open: ${filePath}`)
-    if (mainWindow) {
-        mainWindow.webContents.send("open-file", filePath)
+    if (activeWindow()) {
+        activeWindow().webContents.send("open-file", filePath)
     } else if (process.platform.includes("darwin")) {
         const argsToUse = [...process.argv, filePath]
         createWindow(argsToUse, process.cwd())
@@ -246,8 +254,8 @@ app.on("activate", () => {
     if (!windows.length) {
         createWindow([], process.cwd())
     }
-    if (mainWindow) {
-        mainWindow.show()
+    if (activeWindow()) {
+        activeWindow().show()
     }
 })
 

main/test/WindowManagerTests.ts

@@ -0,0 +1,108 @@
+/**
+ * WindowManagerTests.ts
+ */
+
+import * as assert from "assert"
+import {
+    checkWindowToFindBest,
+    compareDistances,
+    DistanceComparison,
+    windowIsInValidDirection,
+} from "./../src/WindowManager"
+import { Rectangle } from "electron"
+
+describe("checkWindowToFindBest", () => {
+    it("Correctly accepts better window on X axis.", async () => {
+        const currentWindow = { x: 0, y: 0 } as Rectangle
+        const testWindow = { x: 100, y: 0 } as Rectangle
+        const bestWindow = { x: 150, y: 0 } as Rectangle
+        const direction = "right"
+
+        const result = checkWindowToFindBest(currentWindow, testWindow, bestWindow, direction)
+        assert.equal(result, true)
+    })
+    it("Correctly accepts better window on Y axis.", async () => {
+        const currentWindow = { x: 0, y: 0 } as Rectangle
+        const testWindow = { x: 0, y: 100 } as Rectangle
+        const bestWindow = { x: 0, y: 150 } as Rectangle
+        const direction = "down"
+
+        const result = checkWindowToFindBest(currentWindow, testWindow, bestWindow, direction)
+        assert.equal(result, true)
+    })
+    it("Correctly rejects worse window on X axis.", async () => {
+        const currentWindow = { x: 150, y: 0 } as Rectangle
+        const testWindow = { x: 0, y: 0 } as Rectangle
+        const bestWindow = { x: 100, y: 0 } as Rectangle
+        const direction = "left"
+
+        const result = checkWindowToFindBest(currentWindow, testWindow, bestWindow, direction)
+        assert.equal(result, false)
+    })
+    it("Correctly rejects worse window on Y axis.", async () => {
+        const currentWindow = { x: 0, y: 150 } as Rectangle
+        const testWindow = { x: 0, y: 50 } as Rectangle
+        const bestWindow = { x: 0, y: 100 } as Rectangle
+        const direction = "up"
+
+        const result = checkWindowToFindBest(currentWindow, testWindow, bestWindow, direction)
+        assert.equal(result, false)
+    })
+})
+describe("compareDistance", () => {
+    it("Correctly returns lower.", async () => {
+        const currentDiff = 10
+        const bestDiff = 100
+
+        const result = compareDistances(currentDiff, bestDiff)
+        assert.equal(result, DistanceComparison.smaller)
+    })
+    it("Correctly returns higher.", async () => {
+        const currentDiff = 100
+        const bestDiff = 10
+
+        const result = compareDistances(currentDiff, bestDiff)
+        assert.equal(result, DistanceComparison.larger)
+    })
+    it("Correctly returns equal.", async () => {
+        const currentDiff = 10
+        const bestDiff = 10
+
+        const result = compareDistances(currentDiff, bestDiff)
+        assert.equal(result, DistanceComparison.equal)
+    })
+})
+describe("windowIsInValidDirection", () => {
+    it("Correctly accepts window in X axis.", async () => {
+        const currentWindow = { x: 0, y: 0 } as Rectangle
+        const testWindow = { x: 100, y: 0 } as Rectangle
+        const direction = "right"
+
+        const result = windowIsInValidDirection(direction, currentWindow, testWindow)
+        assert.equal(result, true)
+    })
+    it("Correctly accepts window in Y axis.", async () => {
+        const currentWindow = { x: 0, y: 0 } as Rectangle
+        const testWindow = { x: 0, y: 100 } as Rectangle
+        const direction = "down"
+
+        const result = windowIsInValidDirection(direction, currentWindow, testWindow)
+        assert.equal(result, true)
+    })
+    it("Correctly rejects window in X axis.", async () => {
+        const currentWindow = { x: 50, y: 0 } as Rectangle
+        const testWindow = { x: 100, y: 0 } as Rectangle
+        const direction = "left"
+
+        const result = windowIsInValidDirection(direction, currentWindow, testWindow)
+        assert.equal(result, false)
+    })
+    it("Correctly rejects window in Y axis.", async () => {
+        const currentWindow = { x: 0, y: 50 } as Rectangle
+        const testWindow = { x: 0, y: 100 } as Rectangle
+        const direction = "up"
+
+        const result = windowIsInValidDirection(direction, currentWindow, testWindow)
+        assert.equal(result, false)
+    })
+})

main/tsconfig.test.json

@@ -0,0 +1,29 @@
+{
+    "compilerOptions": {
+        "allowUnreachableCode": false,
+        "allowUnusedLabels": false,
+        "experimentalDecorators": true,
+        "forceConsistentCasingInFileNames": true,
+        "jsx": "react",
+        "lib": ["dom", "es2017"],
+        "module": "commonjs",
+        "moduleResolution": "node",
+        "newLine": "LF",
+        "noEmitOnError": true,
+        "noFallthroughCasesInSwitch": true,
+        "noImplicitReturns": true,
+        "noImplicitThis": true,
+        "noUnusedLocals": true,
+        "outDir": "../lib_test/main",
+        "pretty": true,
+        "removeComments": true,
+        "rootDir": ".",
+        "skipLibCheck": true,
+        "strictNullChecks": false,
+        "suppressImplicitAnyIndexErrors": true,
+        "target": "es2015",
+        "sourceMap": true
+    },
+    "include": ["test/**/*.ts"],
+    "exclude": ["node_modules"]
+}

package.json

@@ -163,7 +163,9 @@
         "build:plugin:oni-plugin-markdown-preview": "cd extensions/oni-plugin-markdown-preview && npm run build",
         "build:test": "npm run build:test:unit && npm run build:test:integration",
         "build:test:integration": "cd test && tsc -p tsconfig.json",
-        "build:test:unit": "rimraf lib_test && cd browser && tsc -p tsconfig.test.json",
+        "build:test:unit": "npm run build:test:unit:browser && npm run build:test:unit:main",
+        "build:test:unit:browser": "rimraf lib_test/browser && cd browser && tsc -p tsconfig.test.json",
+        "build:test:unit:main": "rimraf lib_test/main && cd main && tsc -p tsconfig.test.json",
         "build:webview_preload": "cd webview_preload && tsc -p tsconfig.json",
         "check-cached-binaries": "node build/script/CheckBinariesForBuild.js",
         "copy-icons": "node build/CopyIcons.js",
@@ -175,8 +177,9 @@
         "pack:win": "node build/BuildSetupTemplate.js && innosetup-compiler dist/setup.iss --verbose --O=dist",
         "test": "npm run test:unit && npm run test:integration",
         "test:integration": "npm run build:test && mocha -t 120000 lib_test/test/CiTests.js",
-        "test:unit": "npm run test:unit:browser",
-        "test:unit:browser": "npm run build:test:unit && cd browser && electron-mocha --renderer --require testHelpers.js --recursive ../lib_test/browser/test",
+        "test:unit": "npm run test:unit:browser && npm run test:unit:main",
+        "test:unit:browser": "npm run build:test:unit:browser && cd browser && electron-mocha --renderer --require testHelpers.js --recursive ../lib_test/browser/test",
+        "test:unit:main": "npm run build:test:unit:main && cd main && electron-mocha --renderer --recursive ../lib_test/main/test",
         "fix-lint": "npm run fix-lint:browser && npm run fix-lint:main && npm run fix-lint:test",
         "fix-lint:browser": "tslint --fix --project browser/tsconfig.json --exclude **/node_modules/**/* --config tslint.json && tslint --fix --project vim/core/oni-plugin-typescript/tsconfig.json --config tslint.json && tslint --fix --project extensions/oni-plugin-markdown-preview/tsconfig.json --config tslint.json",
         "fix-lint:main": "tslint --fix --project main/tsconfig.json --config tslint.json",