Add "open in new window" option to tabs context menu #134
Conversation
This is still a work in progress... This allows a user to open a file in a new tab. *However* it simply opens the file, so if the file to be opened in a new window has un-saved changes, it will prompt the user to save their changes before it completes.
davidgross
changed the title
|
|
||
| {type: 'separator'} | ||
|
|
||
| {label: 'Open this tab in new window', command: 'tabs:open-tab-in-new-window'} |
There was a problem hiding this comment.
Would you mind title-casing this to match the other commands?
|
The other thing not in this, is a jasmine test to confirm that the button actually works. I have not written tests with that before, it seems that the open window button is not tested in the tree view, which is what I based my "Open Tab In New Window" button on. And a grep in the atom repo didn't result in anything that helped me either(it may be there, I just didn't find it). |
|
Oh, and the name of the button, it feels poorly worded to me, but it was the best I could come up with. If there is a more succinct way of working it, I am happy to change it. |
|
@davidgross I think you can just call it "Open In New Window", since they have already clicked the tab to get the context menu, yes? That would be the same title as is used when right-clicking a file in the Tree View. |
|
@lee-dohm Yeah, that makes sense, doing that now. |
| @@ -24,6 +24,7 @@ class TabBarView extends View | |||
| 'tabs:split-down': => @splitTab('splitDown') | |||
| 'tabs:split-left': => @splitTab('splitLeft') | |||
| 'tabs:split-right': => @splitTab('splitRight') | |||
| 'tabs:open-tab-in-new-window': => @openTabInNewWindow() | |||
There was a problem hiding this comment.
Would you mind renaming this to tabs:open-in-new-window to match the new context menu entry name?
|
Other than the one comment, would you mind adding some specs for this? |
|
@mnquintana Sorry...apparently I am not getting email about this, I just happened to check this page, and see the comments. Do you mean "specs" as in the tests? or specs as in more detail on what this does? ( I assume the tests?) I changed the tabs command name. |
|
@davidgross Sorry, yup, specs meaning tests. Thanks! |
|
@mnquintana tests added, hopefully they work like I think they do. |
davidgross
changed the title
| itemURI = item.getPath() ? '' | ||
| else if typeof item.getUri is 'function' | ||
| itemURI = item.getUri() ? '' | ||
| atom.open({pathsToOpen: [itemURI], newWindow: true}) |
There was a problem hiding this comment.
A tab could have non of above functions (getURI, getPath, getUri). Wouldn't it better to check if itemURI has a value before closing the existing tab and opening a new window with maybe empty paths...
cc @mnquintana
There was a problem hiding this comment.
@jerone did you mean something like this?
openInNewWindow: (tab) ->
tab ?= @children('.right-clicked')[0]
item = tab.item
return unless item?
if typeof item.getURI is 'function'
itemURI = item.getURI() ? ''
else if typeof item.getPath is 'function'
itemURI = item.getPath() ? ''
else if typeof item.getUri is 'function'
itemURI = item.getUri() ? ''
return unless itemURI?
atom.open({pathsToOpen: [itemURI], newWindow: true})
@closeTab(tab)There was a problem hiding this comment.
There was a problem hiding this comment.
The ? '' might be redundant then.
There was a problem hiding this comment.
ok, I stole that from onDragStart.....
updated openInNewWindow path logic to confirm valid path before closing tab and opening new window with that path.
|
Oh, hold it, I mis-read what you said, I thought you were using the 'open in new tab' on the TreeView on the left side. |
|
Ok cool ... was just typing up some new repro steps 😆 |
|
Ok, I am reproducing that. The reason that is happening is I am calling atom.open, and passing two 'pathsToOpen' strings, in an array. see: I am currently seeing if I can make that behavior change.... |
|
ok, so it looks like I would need to be able to set the project path separately, and I am not sure I can do that from the atom.open() call. atom iself or tree-view is trying to be smart, and open the project paths related to the pathsToOpen, so when I pass both, it sets the project as the atom.project.getPaths(), and then adds another project for the parent directory of the itemURI that I am passing.... Thoughts? |
|
Ok, I was afraid of that 😀 I think a modification to |
I think I have this exact same problem in my modular-keymaps package. |
Add "open in new window" option to tabs context menu
|
Thanks for your amazing perseverance @davidgross 😀 |
|
This is a good workaround, but doesn't really fix #39. Please don't forget about this feature! |
|
I agree. Also if you open a new windows and move another tab to the new windows, it does not disappear on the old. But if you split the screen (in a single window) it does. So the moved tab should also disappear. This is a new bug. |
|
I agree with you ( @colepanike and @NicoHood ). However, because of the limitations in Atom itself(not the tabs plugin) that is not possible(or I could not find a way to make it happen), for now, I keep hoping they will add a mouseDragOutOfWindow type event that will allow drag-and-drop-in-new-window type actions, they have added a lot of stuff to the atom api, I think it is getting closer. Also, once you have a new window open, you can drag tabs between windows(you probably already knew that...but there it is anyway). |
|
We then should open a new issue for this feature request and then link issue #39 with this feature. I think you missed my edit while typing your answer. Please reread the 2nd part of my answer :) |
|
@NicoHood indeed :) And you are right about that bug, I'll look into that. |
|
@NicoHood |
|
Go ahead, I think you can give the error description more details than me. |
|
Agree with previous, and also petition that #39 is not solved and should be re-opened. |
|
@colepanike #39 isn't closed. |
|
Ah, so it isn't. Sorry. Should have looked at current state. I confused the referenced thread closure for the closure of the primary thread. |
|
@lee-dohm is there an existing issue for the 'drag-tab-to-other-window-but-not-closing-original-tab' bug? |
|
#39 is locked and no longer open for new comments, so it is not possible to comment there. I am not reading it exactly as I am imaging it so I'd like to take the opportunity to state it here- On occasion I want to expand my open work on to a second screen. So I'd like to grab an open tab and drag it over to a second screen where it will exist in a new instance of Atom on the second screen. Firefox is a good place to see that working as it should. Programmatically this is already nearly being done in that you can drag an open tab in one instance to an open instance of Atom on a second screen. So it seems that the logical extension would be to check if an Atom instance in currently open, if not, open one, and receive the tab. This UX is in recognizing the multi-screen world devs are working in these days. |
|
@riclf that is what this is a work-around for. Atom itself(not the tabs plugin) will need some changes to make drag-tab-out-to-new-window work properly. |
|
This does not work with Markdown Preview :( |
|
I guess its a limitation of this method, but this feature doesn't work if the file is unsaved. |
|
This does not work if you open a new document (ctrl + n) which is not safed yet. It only works for saved docs. |
This creates a workaround for #39
I have added a context menu option for opening a tab in a new window.
I placed it in the "reveal in treeview, reopen closed tab" section, below the split section.
It just spawns a new atom session, and then opens the file in question, and closes it in the current(old) window.
It will prompt the user to save the file before closing the tab in the old window, if there are unsaved changes.