Skip to content

Fix maximizing behaviour#176

Merged
MrRio merged 5 commits intovercel:masterfrom
CompuIves:fix_maximize
Aug 6, 2016
Merged

Fix maximizing behaviour#176
MrRio merged 5 commits intovercel:masterfrom
CompuIves:fix_maximize

Conversation

@CompuIves
Copy link
Contributor

Currently maximizing/minimizing isn't working because of a function call to this.rpc.emit while this.rpc is null.

This PR fixes this with also fixing some minor bugs with minimizing/maximizing.

@rauchg
Copy link
Member

rauchg commented Jul 16, 2016

Thanks for the fix. I think it would be nice if the header didn't emit directly. It'd be cool if you could handle it like we handle other effects.

For example, when you close a tab, we dispatch an action:
https://github.com/zeit/hyperterm/blob/master/app/lib/containers/header.js#L35-L45

Then actions can have side effects in effect: dispatch other actions or rpc calls.
https://github.com/zeit/hyperterm/blob/master/app/lib/actions/ui.js#L20-L35

More info here: https://hyperterm.org/#actions-and-effects

@CompuIves
Copy link
Contributor Author

Ah, I see. I will update the PR!

On Sun, Jul 17, 2016, 00:40 Guillermo Rauch notifications@github.com
wrote:

Thanks for the fix. I think it would be nice if the header didn't emit
directly. It'd be cool if you could handle it like we handle other effects.

For example, when you close a tab, we dispatch an action:

https://github.com/zeit/hyperterm/blob/master/app/lib/containers/header.js#L35-L45

Then actions can have side effects in effect: dispatch other actions or
rpc calls.
https://github.com/zeit/hyperterm/blob/master/app/lib/actions/ui.js#L20-L35

More info here: https://hyperterm.org/#actions-and-effects


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#176 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAj1CBAhm_eda8pTruoRE8KZrM8Nv3wRks5qWV3jgaJpZM4JOFEJ
.

@CompuIves
Copy link
Contributor Author

CompuIves commented Jul 17, 2016

Changed it, I'm still uncertain about one thing. I'm currently using toggle-maximize because in that way I can read from BrowserWindow if the window is maximized or not and handle according to that. I think this is not the cleanest way to do it, since it's not pure. We don't know what will happen if we run the function with the same arguments and we are relying on external logic to determine this.

A solution would be to go back to maximize and unmaximize with the tradeoff that we have to assume the user starts the terminal unmaximized. I'm more in favor of maximize and unmaximize but I first want to know your opinion of this.

@albinekb
Copy link
Contributor

Is it even possible to start an application maximized?

@timothyis timothyis added the 👩‍🔬 Status: In Progress Issue or PR is currently active and work is in progress label Jul 29, 2016
@CompuIves
Copy link
Contributor Author

Updated the PR, decided to store window maximized state in the store and assume we're starting with a maximized state of false.

@MrRio
Copy link
Contributor

MrRio commented Aug 6, 2016

This works really well, can you resolve the conflicts?

@CompuIves
Copy link
Contributor Author

Resolved!

@MrRio MrRio added Status: Completed and removed 👩‍🔬 Status: In Progress Issue or PR is currently active and work is in progress labels Aug 6, 2016
@MrRio MrRio merged commit 749d1f4 into vercel:master Aug 6, 2016
chabou added a commit to chabou/hyper that referenced this pull request Aug 19, 2016
* master:
  chore(package): update react to version 15.3.1 (vercel#637)
  Fix vercel#527: validate cursorColor value and apply default if it fails (vercel#590)
  Added customChildrenBefore to the tabs. (vercel#580)
  Fix for markdown files (vercel#618)
  Provide clear selection of text in terminal view (vercel#608)
  Added shellArgs to the config. (vercel#572)
  Fix international tilde character, and ` and ´ (vercel#584)
  chore(package): update electron-prebuilt to version 1.3.3 (vercel#604)
  chore(package): update should to version 11.0.0 (vercel#602)
  Comments for ignored stuff
  A little shorter
  Unneeded space
  Open new window offset from last focused window (Credit: albinekb) (vercel#581)
  Fix maximizing behaviour (vercel#176)
  Fix mapXDispatch and allow plugin to access onWheel (credit: lkzhao) (vercel#578)
  Use single quotes for better compatibility (vercel#575)
  Add config for bell (vercel#468)
  chore(package): update electron-prebuilt to version 1.3.2 (vercel#553)
  Moved "file-uri-to-path" dep to app package.json (vercel#569)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants