Quick full screen #803

maxdeviant · 2016-10-07T01:44:45Z

This PR is for investigating/adding the quick full screen functionality that @jordwalke requested in #666 (I guess I'm doing the devil's work here 😱 ).

Currently I have it working quickly by just expanding the window to take up all available space on the screen. It feels a little hacky to me, but it is fast 😄

TODO:

Enter quick full screen mode
Leave quick full screen mode
- Restore the window to the previous size/location it was in before entering full screen
Toggle the menu item appropriately
Add accelerators for Linux
~~Figure out if there is a more elegant way to do this~~
- I tried the simple stuff first (i.e., using BrowserWindow.setFullScreen, but that still uses the OS X animations.
- EDIT: After making some tweaks and sleeping on it, I think that my current implementation is probably the best way to go about it, given the constraints of Electron.

jordwalke · 2016-10-07T02:35:42Z

I hope that other electron apps can learn from your approach.

jordwalke · 2016-10-07T02:36:18Z

app/menu.js

+      },
+      {
+        label: 'Enter Quick Full Screen',
+        accelerator: 'Cmd+Enter',


Nice choice! (Is it configurable?)

Currently Hyper doesn't support custom hotkeys. But once we do then it will be configurable!

I don't recall if there is an open issue for that or not.

jordwalke · 2016-10-07T02:37:01Z

app/index.js

@@ -245,6 +246,16 @@ app.on('ready', () => installDevExtensions(isDev).then(() => {
      win.maximize();
    });

+    rpc.on('enter quick full screen', () => {
+      const {width, height} = electron.screen.getPrimaryDisplay().workAreaSize;
+      win.setSize(width, height);


Does this render on top of the menu bar?

Not currently. I was going to ask what you had in mind for the behavior.

maxdeviant · 2016-10-07T02:38:39Z

@jordwalke If it works well I can see about abstracting it into an Electron plugin so other apps can add the same functionality easily 👍

maxdeviant · 2016-10-07T03:13:44Z

Here it is in action: https://gfycat.com/BitterScentedBarnowl

flybayer · 2016-10-07T17:41:35Z

app/menu.js

+      {
+        id: 'LEAVE_QUICK_FULL_SCREEN',
+        label: 'Leave Quick Full Screen',
+        accelerator: 'Cmd+Enter',


Please add an accelerator for Linux users 😀 We don't have Cmd

@flybayer Any thoughts on what the accelerator should be? 🤔 Alt+Enter?

That may be ok, although Ctrl+Shift+Enter might be better. It's similar to Cmd+Shift+F for OSX.

rauchg · 2016-10-08T01:24:12Z

How about ^⌘F ?

rauchg · 2016-10-08T01:24:21Z

(not sure it's better, but seems more standard?)

maxdeviant · 2016-10-08T01:59:05Z

@rauchg ^⌘F is already reserved for normal full screen on OSX (which triggers the animation).

shime · 2016-10-09T13:39:08Z

Great work, can't wait for it to be merged. Worth noting that the hotkey for entering full screen in iTerm is also ⌘↵.

shime · 2016-10-09T16:09:15Z

Detected a couple of issues after using it for longer time:

it remembers the window size after exiting, so when it's restarted it will show in full screen - maybe not a problem, but it's unable to exit full screen as a result
the event is not firing every time the hotkey is pressed, it fires after 2-3 attempts occasionally

maxdeviant · 2016-10-09T22:58:09Z

@shime Yea, I was aware of the first issue, but I'm not totally sure what the best way to handle it is, because Hyper saves your terminal position and size when it exits (as part of another feature).

I was debating between:

Simply not saving the position/size when in quick full screen
- This would restore the position/size you had before entering full screen on the next load
Saving the "is full screen" state to the Electron config (where the position and size are recorded)
- This would bring you back to full screen on the next load, but you would be able to toggle it off using the hotkey

The former is easier since we just have to track the state in-memory, as opposed to the latter where it must be written out when the app exits.

cc @rauchg @jordwalke Any input on how this should work?

maxdeviant · 2016-10-09T23:00:56Z

@shime Also, that second issue is interesting. I've only done cursory testing, so the next time I have a look at it I will see if I can reproduce and fix the issue.

jordwalke · 2016-10-10T00:16:54Z

Me? I'm not picky about which of those two solutions you go for as long as it's not buggy. I'm just happy to have full screen!

rauchg · 2016-10-16T05:19:44Z

lib/index.js

+rpc.on('leave quick full screen req', () => {
+  store_.dispatch(uiActions.leaveQuickFullScreen());
+});
+


I like this "roundtrip" approach. Gives us lots of flexibility

This reverts commit 64e87d0.

* master: chore(package): update electron to version 1.4.5 (vercel#949) add onRendererWindow, because it actually available. check ./lib/utils/plugins.js:193 (vercel#924) increase timeout for update checks (vercel#928) Change URL regex - fixes vercel#867 (vercel#943) Make the close menu items less ambiguous Comply to XO's no-warning-comments rule Prefer default export to make XO happy (vercel#931) chore(package): update ms to version 0.7.2 (vercel#933) chore(package): update copy-webpack-plugin to version 4.0.0 (vercel#927) log plugins' errors in Electron console (vercel#923) Improve tabs title (vercel#892) increase notification timeout up to 30m (vercel#913) Create github templates (vercel#919) chore(package): update electron to version 1.4.4 (vercel#907) Prevent Hterm to share same preferences between instances (vercel#906) 0.8.3 0.9.0 Revert "Quick full screen (vercel#803)" Ignore file watch errors. Fixes vercelGH-225 (vercel#893)

jordwalke · 2016-11-09T22:11:06Z

Did I miss why this was reverted?

maxdeviant · 2016-11-09T22:13:55Z

@jordwalke There was an issue with it where sometimes you would get locked in quick full screen and not be able to get out. Usually it happened if you switched back and forth too fast.

I've been meaning to have a look at it, but haven't had any time to devote to it lately 🙁

hojberg · 2017-01-26T15:00:16Z

damn i was excited about this

ChadTaljaardt · 2017-02-16T00:38:20Z

Bump for this, it is a must have feature for productivity in my opinion.

asantos00 · 2017-09-25T12:52:57Z

Any plans about the Cmd+enter action to fullscreen on mac?

albinekb · 2017-09-25T13:04:52Z

you can do that with the custom keymaps @asantos00

asantos00 · 2017-09-25T15:08:39Z

So it will not be added as default behaviour? I think it is default behaviour for all OSX apps :(

maxdeviant added 3 commits October 6, 2016 21:37

Add pseudo quick full screen

f607c0c

Record window size before entering full screen

f5165c0

Restore window size when leaving full screen

e3bc472

jordwalke reviewed Oct 7, 2016

View reviewed changes

Toggle menu items for quick full screen

7779beb

maxdeviant changed the title ~~[WIP] Quick full screen~~ Quick full screen Oct 7, 2016

Remove redundant filters

986721d

flybayer reviewed Oct 7, 2016

View reviewed changes

maxdeviant added 2 commits October 7, 2016 20:00

Merge remote-tracking branch 'zeit/master' into quick-full-screen

ae6a4dd

Add accelerators for Linux

2a326e5

maxdeviant added 2 commits October 7, 2016 22:42

Merge branch 'master' into quick-full-screen

a4ad8bc

Merge branch 'master' into quick-full-screen

c5d41f5

maxdeviant added 3 commits October 11, 2016 23:41

Merge branch 'master' into quick-full-screen

34df15a

Only save window state when not full screen

7c1eaf8

Merge remote-tracking branch 'zeit/master' into quick-full-screen

1023bcc

rauchg approved these changes Oct 16, 2016

View reviewed changes

rauchg merged commit 64e87d0 into vercel:master Oct 16, 2016

rauchg added a commit that referenced this pull request Oct 18, 2016

Revert "Quick full screen (#803)"

2ebb236

This reverts commit 64e87d0.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quick full screen #803

Quick full screen #803

maxdeviant commented Oct 7, 2016 •

edited

Loading

jordwalke commented Oct 7, 2016

jordwalke Oct 7, 2016

maxdeviant Oct 7, 2016

jordwalke Oct 7, 2016

maxdeviant Oct 7, 2016

maxdeviant commented Oct 7, 2016

maxdeviant commented Oct 7, 2016

flybayer Oct 7, 2016

maxdeviant Oct 7, 2016

maxdeviant Oct 7, 2016

flybayer Oct 7, 2016

rauchg commented Oct 8, 2016

rauchg commented Oct 8, 2016

maxdeviant commented Oct 8, 2016

shime commented Oct 9, 2016 •

edited

Loading

shime commented Oct 9, 2016

maxdeviant commented Oct 9, 2016 •

edited

Loading

maxdeviant commented Oct 9, 2016

jordwalke commented Oct 10, 2016

rauchg Oct 16, 2016

jordwalke commented Nov 9, 2016

maxdeviant commented Nov 9, 2016

hojberg commented Jan 26, 2017

ChadTaljaardt commented Feb 16, 2017

asantos00 commented Sep 25, 2017

albinekb commented Sep 25, 2017

asantos00 commented Sep 25, 2017

Quick full screen #803

Quick full screen #803

Conversation

maxdeviant commented Oct 7, 2016 • edited Loading

TODO:

jordwalke commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxdeviant commented Oct 7, 2016

maxdeviant commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rauchg commented Oct 8, 2016

rauchg commented Oct 8, 2016

maxdeviant commented Oct 8, 2016

shime commented Oct 9, 2016 • edited Loading

shime commented Oct 9, 2016

maxdeviant commented Oct 9, 2016 • edited Loading

maxdeviant commented Oct 9, 2016

jordwalke commented Oct 10, 2016

Choose a reason for hiding this comment

jordwalke commented Nov 9, 2016

maxdeviant commented Nov 9, 2016

hojberg commented Jan 26, 2017

ChadTaljaardt commented Feb 16, 2017

asantos00 commented Sep 25, 2017

albinekb commented Sep 25, 2017

asantos00 commented Sep 25, 2017

maxdeviant commented Oct 7, 2016 •

edited

Loading

shime commented Oct 9, 2016 •

edited

Loading

maxdeviant commented Oct 9, 2016 •

edited

Loading