Skip to content
This repository has been archived by the owner on Jun 11, 2021. It is now read-only.

Fix for menu bar not displaying in windows #1559

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

Bigguy34
Copy link
Contributor

So I spent a couple days on this, and there are a couple problems. Firstly the main reason that the menu bar doesn't display in windows is because kitematic sets frame:false in the settings of the browser window. This prevents it from being displayed in Windows. So I implemented a fix. By removing the frame:false and changing it to frame: os.platform() === 'win32', stuff started working. But there are a lot of UI problems. I tried to fix most of them. Here is a screen shot of what I have so far(And yes hiding the bar is supported with the alt key).
menubar

Here is window before the alt key is pressed.
withoutmenubar

The biggest problem is that I had to get rid of the header with the login and logo. I plan on putting back the information, I just want to make sure first that this is worth pursuing. If it is I would be curious to hear from some of the maintainers where the best place to put the rest of this information(I was thinking of putting it in top menu bar, or moving it to the bottom some where). I would be curious to here some thoughts. Thanks!

Edit: Adding enhancement reference #860

@pmario
Copy link

pmario commented Mar 20, 2016

Hi. I couldn't figure out, why it didn't work. So thx for starting this PR. ... I also did read a lot of the docs, but didn't see this frame:true setting. Can you point me to the right place?

I'll test your changes soon.

@Bigguy34
Copy link
Contributor Author

Ok another update on this PR. I tried in on my mac, and it actually looks ok. I also had a dependency that wasn't working so I got rid of that. @pmario Here are the frameless window docs . Here is screenshot from my OSX.
image

@Bigguy34
Copy link
Contributor Author

Also I am going to breakout the menu into two different templates, one for Windows and one for OSX. As I look closer at the OSX menu(which still works) there are some things we just don't need in windows that are needed in OSX.

@pmario
Copy link

pmario commented Mar 21, 2016

Hi, I just tested you PR. Looks good so far. .. Just most commands don't work on my machine. eg: hide, ..

IMO also the menu structure needs to be changed. eg:

Kitematic
  Hide Kitematic
  Hide Others
  Show All
  ----
  Quit
Edit
   .. is ok ..
View
   Search / Main View / Home ... (Same functionality as the [+ New] button in the sidebar)
   Container Home
   Container Settings
Tools
   Preferences
   Open Docker Terminal
   ----
   Toggle Dev Tools.
Help
   Online Documentation  (Link)
   Report Issue - Feedback
   ----
   About

Topics below would be completely new feature so imo "File" and "Window" can be removed.

File    
   Open Profile
   Save Profile

@Bigguy34
Copy link
Contributor Author

Yea I agree. Some of these menu's aren't needed, I also need to fix the issue with login info. Thanks for giving it a try. @FrenchBen @JeffDM any thoughts on this PR?

@Bigguy34
Copy link
Contributor Author

Has anyone seen this blog post. Will kitematic still be relevant? Does that mean kitematic won't use virtualbox? @pmario I am still trying to find time to work on this, possibly today I will be able to take another look.

@pmario
Copy link

pmario commented Mar 24, 2016

I think, docker for Mac and Windows is a different approach. IMO it's just a tighter integration into the native OS. In the video there is not much UI. Windows users will need a lot of UI and no CLI at all! Command line freaks the hell out of us ;).

For windows you'll need hyper-v atm. ... Which is a good thing for me. I currently hacked kitematic to support hyper-v instead of vbox. .. You can have only one active hyper visor in windows. So if hyper-v is active vbox can't work anymore. .... but

Probably the minority of windows users have OS versions installed, that support hyper-v. So vbox imo will always be a fast and cheap option for the masses.

I think there is much need for a program like kitematic for beginners. You basically have a "One click" experience, with a fast success. It abstracts all the glitches away, which is good at the beginning. ...

After a while, as you'll start to dig deeper. You'll need more fine grained control, where the CLI comes back in. .. Or a much improved (may be rewritten) future Kitematic. eg: Kitematic is english only atm. ...

@Bigguy34
Copy link
Contributor Author

Ok I have been working on this, and have it working for the most part, but I really wanted to write some tests to make sure the behavior was working properly on each menu item. But I keep hitting my head on the outdated react-router. It seems like from googling for 20 minutes that older version of react-router really struggle with supporting the mocking in jest. Has anyone tried updating react-router? It looks like the latest react-router npm package is at version 2. For now I think I am going to stub out the router just so I can have awesome tests. And then create a different PR for updating react-router. Thoughts?

@pmario
Copy link

pmario commented Apr 4, 2016

@Bigguy34 anything to share?

… need to finish testing on OSX, and add tests for OSX.

Signed-off-by: Thomas Florin <thoflo002@gmail.com>
Signed-off-by: Thomas Florin <thoflo002@gmail.com>
@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Apr 5, 2016

Sorry @pmario it took me a really long time to get the jest tests to work, but in the long run I think it will pay off. I still need to refactor the header, what I have right now is a bad hack for the header. I would be curious to hear some feedback on my menu builder design. I will try and have the header finished with all the information that was there before, although the UI may need to be looked at.

@FrenchBen
Copy link
Contributor

Thanks @Bigguy34 for the work and @pmario for the review - Super helpful to have more Windows guy checking things.
Also @Bigguy34 don't hesitate to squash your commits when appropriate. Helps keep the master branch's history clean

…r it.

Signed-off-by: Thomas Florin <thoflo002@gmail.com>
@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Apr 6, 2016

Ok here is what this looks like right now in Windows. Sorry my next commit will squash the commits. Here is what the UI looks like in windows. Any suggestions to make this look better? @FrenchBen, @pmario? Once I get this solidified then I will test it out in OSX.
image

@pmario
Copy link

pmario commented Apr 6, 2016

@Bigguy34 I did test the later version and it looks good. I did merge it with my hyper-v stuff, which works great together for me :)...

The only thing, that didn't work was "minify window". imo the window menu could be skipped at all. The close Window should be the "Exit" command and minify is a toolbar button. Ctrl-W imo isn't a good command. Since it's too easy to hit by accident. Windows has a native Alt-F4 for this.

just some thougts.

@FrenchBen
Copy link
Contributor

  • +1 on the Alt+F4 to close it.
  • +1 on minify being a toolbar button

@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Apr 6, 2016

Ok I will fix those buttons. I actually have thought about this a lot, and I think I am going to try a different direction with the UI. I really want to maintain the current UI layout. I think the best way to do that is to keep the frame:false but conditionally(windows or not) add a custom React component that looks like and acts like a menu. I will post a screenshot as soon as I have a prototype working. Sorry guys this is taking so long, I really want the UI to look right.

@FrenchBen
Copy link
Contributor

Couple of additional notes - Could we get the kitematic word to be clickable as well?
Also it seems that the menu is always aligned-right for me?

@FrenchBen
Copy link
Contributor

@pmario another dev also took on the challenge, as seen in this PR:
#1835

Not sure which one should be kept. WDYT?

@Bigguy34
Copy link
Contributor Author

@pmario I will work on this tomorrow, I am just getting my branches up to date to be able to finish it up. Thoughts @FrenchBen? I am going to make it look more like #1835

@pmario
Copy link

pmario commented Aug 10, 2016

@FrenchBen

@pmario another dev also took on the challenge, as seen in this PR: #1835

yea, that was the trigger to ping this topic again :)

Not sure which one should be kept. WDYT?

I'd be happy with "whatever works" - menu wise. ...

OT: I personally don't use "maximize" window button. There are much faster ways to do what I want.

@FrenchBen
Copy link
Contributor

Let's keep this PR as the proper fix and go with full frames, instead of mocking with it to see what we get and hopefully simply some of the code.

@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Aug 11, 2016

Ok, I still need to test it one more time in Windows just to make sure, but it is starting to look pretty good, and I quickly updated the code for OSX, I also need to put some polish around logged in vs not logged in. Still have a little more work to do before someone can test it. I'll post some screen shots soon.

@Bigguy34
Copy link
Contributor Author

Here is what it looks like in OSX, this shouldn't come as surprise, an important note here is that close/minimize/fullscreen are native, and not html elements.
screen shot 2016-08-15 at 9 35 53 pm

@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Aug 16, 2016

Windows is where the UI starts to get wonky, here is how I currently have the ui layed out in Windows. I tried removing the native logo at the very top the window, but electron will just default to the executable ico if one isn't specified. One hack would be to just use a 1x1 white ico, that might look a little better. Thoughts @FrenchBen? Also @pmario are you saying that Exit should be a top level menu option and I should just remove minimize?(From your comment you made a couple months ago)
kitematic

Also as a sidenote, I noticed in this PR in the screenshot for OSX he has a much different looking native window then I do. Is this because it is an older version of OSX?

@pmario
Copy link

pmario commented Aug 16, 2016

@Bigguy34
It seems you'r using a very old master branch? ... So ctrl-shif-t ... Terminal doesn't work. This should be fixed in master. So I don't know if this is a new issue or just because your version is based on an old master.

For me ctrl - m and Window: Minimize don't work. I'm using win10pro 1607

@pmario
Copy link

pmario commented Aug 16, 2016

For me the help menu should look like this:

Help
   Online Documentation ... linked to: https://docs.docker.com/kitematic/userguide/
   Report Issue ... as is
   Online Feedback  ... Linked to: https://forums.docker.com/c/open-source-projects/kitematic
   ----
   About

As in general. The existing menu system may be ok for the Mac, but it feels completely wrong in windows (at least for me). ... you know windows is different ;)

@Bigguy34
Copy link
Contributor Author

Ok I will update it for Windows, and include the terminal hot key. As well as the update recently for the new OSX hockey, that I saw come across my feed.

@pmario
Copy link

pmario commented Aug 24, 2016

hi. I'll test it, at soon as you push the new stuff. I think, there is a little bit too much "branding" going on with the windows version. ... but this can be fixed in a new PR, because it's just cosmetics :)

@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Aug 25, 2016

Ok I updated the menus to have better windows support, @pmario I will merge from upstream which should hopefully fix the out of date issues sorry.

@pmario
Copy link

pmario commented Aug 25, 2016

I did some tests with windows. looks good to me. ... will need some tests with linux (ubuntu 16.04) too. .. I'll post my results. have fun!

@Bigguy34
Copy link
Contributor Author

Bigguy34 commented Sep 1, 2016

Ok thoughts? Sorry things got confusing for a little bit there as I was merging, but I think I have everything worked out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants