Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Add new window option #1111

Merged
merged 3 commits into from
Dec 13, 2017
Merged

Add new window option #1111

merged 3 commits into from
Dec 13, 2017

Conversation

akinsho
Copy link
Member

@akinsho akinsho commented Dec 12, 2017

@bryphe following the discussion in #1100 and the link you posted I had a go at adding the functionality of opening a new window to the top menu bar (and to the dock for mac os). Tbh I was little concerned that implementing this would be more involved, i.e. managing the state of multiple windows etc. but it all seems to work fine (which I believe is largely due to your foresight in handling focusing of instances etc. in main.ts).

Things that it currently lacks that I imagine might become issues down the line are

  1. mappability
  2. an easy way to focus instances
  3. cmd-Q and I imagine its alternate OS equivalents kills all windows, subjectively I would have expected each to be deleted one at a time - looking into this

Tbh I personally think those are both nice to haves and the menu or clicking each instance works well, and mac os, linux and i'd imagine windows all have several window manager programs, but perhaps thoughts for the future

@bryphe
Copy link
Member

bryphe commented Dec 12, 2017

Awesome, thanks for looking at this @Akin909 ! Impressive turnaround time on the PR 😄

I just tried it out, and it's working great!

Agree with your list of items for 1-3, and that they could potentially be handled down the road. It's great that we have the 'New Window' option.

cmd-Q and I imagine its alternate OS equivalents kills all windows

I believe this is actually the 'right' behavior on OS X - there was some discussion around this on #483 . And when I tried on other apps (atom, vscode), it did that full-close behavior.

For 2, one item I was looking at was this: #67

It's titled "multi-monitor support" but really it's just about quickly switching open instances of Oni. I thought it'd be cool if we could extend the window navigation commands (like <C-w>h) such that, when they hit a border, they would look to see if there is an Oni window present to the left/right/top/bottom, and navigate to that instance. Another option might be to have a quick-open style menu for open instances that could be navigated. But as you mentioned each platform also has their own ways to navigate windows, so it's a nice to have.

Nice work! I'll bring this in once the builds are complete.

@akinsho
Copy link
Member Author

akinsho commented Dec 12, 2017

@bryphe 👍 . Re. windows closing if that standard behaviour i'll leave that be.
As for window jumping that would be a really cool feature to use vim-esque key maps to move between windows, I saw somewhere in the docs that windows have x,y positions perhaps looking at those i.e comparing the current positions x to all the other windows in the array and if otherWindow.x > currentWindow.x then focus on that instance then map that to the appropriate key, just a thought.
Another thing I noticed was that you can add recent files to the menu icon in mac and windows thought it isn't clear how to populate the recent files from the docs

@bryphe
Copy link
Member

bryphe commented Dec 13, 2017

As for window jumping that would be a really cool feature to use vim-esque key maps to move between windows, I saw somewhere in the docs that windows have x,y positions perhaps looking at those i.e comparing the current positions x to all the other windows in the array and if otherWindow.x > currentWindow.x then focus on that instance then map that to the appropriate key, just a thought.

Definitely! The browser window has a getBounds method which gives us the positions, and the main.ts has that list of all the browser windows.... so it'd be a matter of figuring out how to go from one to the other.

I started on some ideas here a looong time ago - in the main process, I had this code to focus different instances:

ipcMain.on("focus-next-instance", () => {

And then the entry point, from the browser-side, was really simple here - just sending the ipc message:

public focusPreviousInstance(): void {

So I think it wouldn't be too crazy to implement the navigation - we'd just have to tell the main process which direction we want to go, and then in the main process, see if there is an Oni window that matches (like, if we move right, check for that otherWindow.x > currentWindow.x as you mentioned). Doesn't seem like it'd be too far out!

Another thing I noticed was that you can add recent files to the menu icon in mac and windows thought it isn't clear how to populate the recent files from the docs

Ah interesting, I guess this might be a case where having those 'persisted' settings would be helpful - we could persist the last X opened files/folders, and show them there? I wonder if there is something special about them, or if they would be additional menu items we'd add to our dock menu?

@bryphe
Copy link
Member

bryphe commented Dec 13, 2017

Anyway I'll go ahead and bring this in as it addresses #1111 . Thanks again for your work on this, @Akin909 !

@bryphe bryphe merged commit 40e54a6 into onivim:master Dec 13, 2017
@akinsho akinsho deleted the add-new-window-option branch December 13, 2017 00:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants