Split Panes #693

ekmartin · 2016-09-06T04:18:51Z

This adds an initial implementation of split panes, as shown in the screenshot below.

Features

Split panes can be created by hitting Cmd+D and Cmd+Shift+D (which are also accessible from the Window menu).
Split panes can be arbitrarily nested, and support tabs like you'd expect.
The active pane can be changed with mouse clicks, or by cycling through them using Show Next Pane/Show Previous Pane.
Panes can be closed with CmdOrCtrl+W.

How

There's definitely lots of ways to implement something like this, however I've opted for a pretty Redux heavy direction. The whole split pane structure is stored in a new reducer, term-groups.js, which reacts to create/remove/focus change actions and adapts the structure as needed.

Reducer Structure

The term-groups reducer maintains a tree like structure of the different tabs.
The split structure in the screenshot would resemble something like this:

   +-----+Root+----+
   |               |
   v               v
++Left++       ++Right++
|      |       |   +   |
v      v       v   v   v
S      S       S   S   S

Each term group would have the following properties:

uid - group identifier
parentUid - the parent's uid
sessionUid - if the group is a leaf node, conforms to the actual session's uid
direction - what direction the split is created in (only for internal nodes)
children - an array of uids of the term group's children.

Additionally, the reducer holds a mapping between each root term group and their respective active sessionUid, so that you can change tabs without losing the focus of the session you'd selected.

Caveats

To be able to create new term groups the SESSION_ADD action is hijacked, and the split direction (if it's a split that caused the new session to be created) is added here. This feels a bit dirty, and I'd love suggestions on how this could be improved (and suggestions on how to improve the data structure in general!).

Rendering

The actual split panes are rendered using react-split-pane at the moment, but this probably needs to be changed before this is ready for primetime. A few issues:

It doesn't support more than two split panes out of the box without nesting split panes (workaround).
The styling is a slightly awkward. This could be fixed with an upstream pull request to allow custom class names for the pane divider (resizer), so we could use it with Aphrodite.
Mouse resizing also seems a bit glitchy, but this might be an issue on our end as well - haven't spent too much time on it yet.

We could either look into fixing these issues upstream, or just implement our own flexbox heavy approach.

TODOs

✅ Preserving already created hterm instances

At the moment we're creating hterm instances at the bottom of the React hierarchy, in the Term component. This doesn't work very well with split panes, since Terms will be removed and recreated when the split pane structure is changed. That means the same happens to their respective hterm instances, which means that sessions lose their output when splitting (i.e. needs to be fixed before splits are usable).

One of the ways of solving this would be to create hterm instances at a higher level, and then pass them down to the Term component. This could either be in a React component such as Terms, or even in a Redux reducer. The latter may sound a bit crazy, but I think it might make sense together with the current reducer structure. I'm interested in more ideas around this though.

✅ Hotkeys

This definitely needs some bikeshedding. Even if we need a way of configuring hotkeys in the future, sane defaults will always be important. As mentioned splits are currently created with CtrlOrCmd+D and CtrlOrCmd+Shift+D. Cmd+D is definitely okay, but Ctrl+D obviously isn't 😙. Maybe we can change it based on the user's OS. What would be a decent hotkey set for Linux?

There also isn't a hotkey for rotating back and forth between your open split panes yet. iTerm defaults this to Cmd+[/Cmd+], however this doesn't work on a lot of European keyboards. Personally I'd prefer something like Ctrl+Tab/Ctrl+Shift+Tab, which we've currently set to changing tabs (afaik it doesn't work though, the combination is eaten by hterm).

EDIT: Cycling between split panes is currently bound to Ctrl+Alt+Tab and Ctrl+Shift+Alt+Tab.

In the future we should definitely also support changing panes based on direction (Cmd+Alt+Arrow in iTerm).

✅ Resizing

Drag and drop resizing of split panes sort of works at the moment, but it's really glitchy.

✅ Styling

Needs to support custom styling in the same way as the rest of HyperTerm. We could probably add some sort of indicator to show what pane is in focus (i.e. an overlay over the inactive panes, like iTerm) as well.

Bugs

Clicking to change focus doesn't work when applications such as vtop/htop are open.
Splitting panes will make git status output look funky (and probably other resizing issues) - screenshot.
A tab will sometimes crash after a pane is closed, needs more investigation.
Closing the last tab won't exit HyperTerm (regression).
Creating multiple horizontal splits is buggy.
Webviews don't respect focus changes as they should.

...and probably a lot more!

Binary

I really recommend pulling down the branch, however I've made a binary that can be downloaded here: https://github.com/ekmartin/hyperterm/releases/download/0.8.0-splits.5/mac.zip (updated 2016-10-02)

This allows users to split their Hyperterm terms into multiple nested splits, both vertical and horizontal. Fixes vercel#56

New split panes should be placed after the currently active pane, not at the end like they were previously.

This adds menu buttons for moving back and forward between open split panes in the currect terminal tab. Doesn't add a hotkey yet, needs some bikeshedding.

It made little sense to have so many objects with `activeSessionUid` set to `null` when it only mattered on the top level. Now it's an object mapping term-group `uid` to `sessionUid` instead.

matheuss · 2016-09-06T13:40:33Z

THAT'S FREAKING AWESOME

chabou · 2016-09-07T21:05:54Z

Great work !

I found a little bug when you reduce the height of a horizontally splitted tab, the bottom one keeps its original size.
But it seems to be a react-split-pane bug : if I add display: flexto the second Pane div, it fixes its size.
Maybe related to tomkp/react-split-pane#106

How can we help you with this PR ?

ekmartin · 2016-09-08T02:39:48Z

Thanks!

@chabou I think the main steps in moving forward will be deciding on what the correct direction to go regarding actions/reducers is, and what to do with the hterm instance issue I mentioned above.

I'm going to be looking into what to do with react-split-pane, probably by seeing if reimplementing some of the functionality directly in HyperTerm might be better than using it.

If you're interested in helping out with some of the bugs that need to be squashed before it's ready that would be helpful. Quick summary:

Clicking to change focus doesn't work when applications such as vtop/htop are open.
Normally this is done by attaching a focus listener here, however this is never fired when you for example click on a term that has htop open.
~~A tab will sometimes crash after a pane is closed.~~ (fixed)
Hotkeys for cycling back and forth between split panes.

Sometimes the terminal would crash when a specific split pane was closed, because the `activeSessions` mapping wasn't updated correctly.

iamstarkov · 2016-10-03T13:34:53Z

@ekmartin @rauchg thank you for all the work done in this issue.
Can you share some news/plans on this pr?

ekmartin · 2016-10-03T15:13:26Z

@iamstarkov: We found a few bugs with multiple horizontal (Y-directional) split panes, we're looking into that now :)

austinyearlykim · 2016-10-03T16:40:26Z

It's crazy seeing a vital feature like this in progress in real time right now. I'M SO STOKED

- open google.com on the main window - open a new tab - come back to previous tab. webview is gone :|

flybayer

Feedback from Ubuntu 14.04

Running exit freezes HyperTerm
I propose changing the new split keyboard shortcuts to these. I've been using these in Vim/Tmux for awhile and love it! Before, I could never remember which was vertical or horizontal.
- Split vertically: Cmd/Ctrl + \
- Split horizontally: Cmd/Ctrl + -

flybayer · 2016-10-03T20:12:48Z

I just realized I'm testing zet:add/splits instead of ekmartin:splits. Which branch should we be testing?

maxdeviant · 2016-10-03T20:39:05Z

@flybayer Doesn't Cmd/Ctrl + - conflict with zoom out?

flybayer · 2016-10-03T20:49:37Z

@maxdeviant oops yes 😆 I guess I'm actually using Meta + - and Meta + \ and forgot the difference. Is Meta + - and Meta + \ a viable default for this?

ekmartin · 2016-10-03T23:43:37Z

@flybayer: the exit bug should be fixed with b2ca957 :)

ekmartin · 2016-10-04T00:40:00Z

@flybayer: I added the split hotkeys Ctrl+Shift+O (horizontal) and Ctrl+Shift+E (vertical) - stolen from Gnome Terminator. If there's better options we can open an issue and do some bikeshedding after this has been merged :)

iamstarkov · 2016-10-04T06:35:56Z

🎉 yay 🎉

iamstarkov · 2016-10-04T06:37:28Z

any plans to release?

naim · 2016-10-19T18:42:46Z

This would be really nice to get into mainline.

Edit: Nevermind! I missed the fact this actually was merged (just saw the labels).

chabou · 2016-10-19T18:55:27Z

I think it is included in 0.8.1 release https://github.com/zeit/hyper/releases/tag/0.8.1

iamstarkov · 2016-10-19T20:33:21Z

@naim @chabou thats true

iamstarkov · 2016-10-19T20:34:22Z

@naim though you need to download hyper. before it was hyperterm and has been renamed

chabou · 2016-10-19T22:12:08Z

In Progress and Review Needed labels seem confusing 😄

* npm: add .npmrc with save-exact=true * split panes: create initial implementation This allows users to split their Hyperterm terms into multiple nested splits, both vertical and horizontal. Fixes vercel#56 * split panes: suport closing tabs and individual panes * split panes: ensure new splits are placed at the correct index New split panes should be placed after the currently active pane, not at the end like they were previously. * split panes: add explicit dependency to uuid * split panes: implement split pane cycling This adds menu buttons for moving back and forward between open split panes in the currect terminal tab. Doesn't add a hotkey yet, needs some bikeshedding. * split panes: move activeSessionUid to its own object It made little sense to have so many objects with `activeSessionUid` set to `null` when it only mattered on the top level. Now it's an object mapping term-group `uid` to `sessionUid` instead. * split panes: make sure closing the last split pane exits the app * split panes: fix a crash after closing specific panes Sometimes the terminal would crash when a specific split pane was closed, because the `activeSessions` mapping wasn't updated correctly. * split panes: fix a bug that caused initial session sizing to be wrong * fix all our focus / blur issues in one fell swoop :O (famous last words) * get rid of react warning * hterm: make sure not to lose focus when VT listens on clicks * term: restore onactive callback * add missing `return` to override (just in case) * split pane: new split pane implementation * goodbye react-split-pane * added term group resizing action and reducer * terms: supply border color so that we can use it for splits * term-group: add resizing hook * term-groups: add resizing constant * remove split pane css side-effect * split panes: pass existing hterm instances to Term * split panes: add keybindings for split pane cycling * split panes: remove unused action * split panes: remove unused styling * split-pane: remove `console.log` * split-pane: remove `console.log` * split panes: rebalance sizes on insert/removal * split panes: pass existing hterm instances to Term * split panes: add keybindings for split pane cycling * split panes: remove unused action * split panes: remove unused styling * split panes: rebalance sizes on insert/removal * split panes: set a minimum size for resizing * split-pane: fix vertical splits * css :| * package: bump electron * split panes: attach onFocus listener to webviews * 1.4.1 and 1.4.2 are broken. they have the following regression: - open google.com on the main window - open a new tab - come back to previous tab. webview is gone :| * split panes: handle PTY exits * split panes: add linux friendly keybindings

ekmartin added 7 commits September 5, 2016 01:35

npm: add .npmrc with save-exact=true

1a28d2a

split panes: create initial implementation

02c3f60

This allows users to split their Hyperterm terms into multiple nested splits, both vertical and horizontal. Fixes vercel#56

split panes: suport closing tabs and individual panes

67669b0

split panes: ensure new splits are placed at the correct index

beb2037

New split panes should be placed after the currently active pane, not at the end like they were previously.

split panes: add explicit dependency to uuid

72fb7d7

split panes: implement split pane cycling

3af3dd9

This adds menu buttons for moving back and forward between open split panes in the currect terminal tab. Doesn't add a hotkey yet, needs some bikeshedding.

split panes: move activeSessionUid to its own object

4ec7892

It made little sense to have so many objects with `activeSessionUid` set to `null` when it only mattered on the top level. Now it's an object mapping term-group `uid` to `sessionUid` instead.

This was referenced Sep 6, 2016

Split panes #56

Closed

Reducer specs #150

Closed

matheuss added the Status: Review Needed label Sep 6, 2016

ekmartin and others added 17 commits September 7, 2016 22:55

split panes: make sure closing the last split pane exits the app

f2e10ca

split panes: fix a crash after closing specific panes

eb0d7b3

Sometimes the terminal would crash when a specific split pane was closed, because the `activeSessions` mapping wasn't updated correctly.

split panes: fix a bug that caused initial session sizing to be wrong

2beb9ba

Merge branch 'master' into splits

60f0e6b

fix all our focus / blur issues in one fell swoop :O (famous last words)

c74c664

get rid of react warning

2b18af8

hterm: make sure not to lose focus when VT listens on clicks

a431504

term: restore onactive callback

6ec526b

add missing return to override (just in case)

034a7b6

split pane: new split pane implementation

2ae9e3a

goodbye react-split-pane

080c394

added term group resizing action and reducer

1b0e598

terms: supply border color so that we can use it for splits

f95a548

term-group: add resizing hook

519a0f8

term-groups: add resizing constant

7591901

remove split pane css side-effect

86906ab

split panes: pass existing hterm instances to Term

c4a6fdf

split-pane: fix vertical splits

860624c

ekmartin force-pushed the splits branch from 202b046 to c0c6a21 Compare October 3, 2016 15:35

rauchg and others added 5 commits October 3, 2016 10:40

css :|

620f8ca

package: bump electron

9939645

Merge branch 'add/splits' into splits

07df51d

split panes: attach onFocus listener to webviews

e0a8581

1.4.1 and 1.4.2 are broken. they have the following regression:

5191cf2

- open google.com on the main window - open a new tab - come back to previous tab. webview is gone :|

flybayer reviewed Oct 3, 2016

View reviewed changes

split panes: handle PTY exits

b2ca957

split panes: add linux friendly keybindings

fea7e79

Merge branch 'master' into splits

9bcc15c

rauchg merged commit a7595c1 into vercel:master Oct 4, 2016

ekmartin deleted the splits branch October 4, 2016 02:50

zadjii-msft mentioned this pull request Apr 28, 2020

Request: Right-click menu inside TerminalControl (w/ Copy & Paste?) microsoft/terminal#3337

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split Panes #693

Split Panes #693

ekmartin commented Sep 6, 2016 •

edited

matheuss commented Sep 6, 2016

chabou commented Sep 7, 2016

ekmartin commented Sep 8, 2016 •

edited

iamstarkov commented Oct 3, 2016

ekmartin commented Oct 3, 2016

austinyearlykim commented Oct 3, 2016

flybayer left a comment

flybayer commented Oct 3, 2016

maxdeviant commented Oct 3, 2016 •

edited

flybayer commented Oct 3, 2016

ekmartin commented Oct 3, 2016

ekmartin commented Oct 4, 2016

iamstarkov commented Oct 4, 2016

iamstarkov commented Oct 4, 2016

naim commented Oct 19, 2016 •

edited

chabou commented Oct 19, 2016

iamstarkov commented Oct 19, 2016

iamstarkov commented Oct 19, 2016

chabou commented Oct 19, 2016

Split Panes #693

Split Panes #693

Conversation

ekmartin commented Sep 6, 2016 • edited

Features

How

Reducer Structure

Caveats

Rendering

TODOs

✅ Preserving already created hterm instances

✅ Hotkeys

✅ Resizing

✅ Styling

Bugs

Binary

matheuss commented Sep 6, 2016

chabou commented Sep 7, 2016

ekmartin commented Sep 8, 2016 • edited

iamstarkov commented Oct 3, 2016

ekmartin commented Oct 3, 2016

austinyearlykim commented Oct 3, 2016

flybayer left a comment

Choose a reason for hiding this comment

Feedback from Ubuntu 14.04

flybayer commented Oct 3, 2016

maxdeviant commented Oct 3, 2016 • edited

flybayer commented Oct 3, 2016

ekmartin commented Oct 3, 2016

ekmartin commented Oct 4, 2016

iamstarkov commented Oct 4, 2016

iamstarkov commented Oct 4, 2016

naim commented Oct 19, 2016 • edited

chabou commented Oct 19, 2016

iamstarkov commented Oct 19, 2016

iamstarkov commented Oct 19, 2016

chabou commented Oct 19, 2016

ekmartin commented Sep 6, 2016 •

edited

ekmartin commented Sep 8, 2016 •

edited

maxdeviant commented Oct 3, 2016 •

edited

naim commented Oct 19, 2016 •

edited