Make html menus scroll to avoid truncation

[redmunds]

This is for #7496

This is an alternate solution for #7693. This isolates the change to only HTML main menus.

Using a max-height such as calc(100vh - 34px) (which would need to be something like calc(~"100vh - 34px") in LESS) was discussed, but doesn't seem to work. I don't think it really that important to have the menu go exactly to the bottom of the screen, anyway.

This solution does not resize the menu while it is open on window resize, but it will be properly resized on next open after resize is complete.

I think this is a reasonable solution to prevent HTML menu truncation for now, and we can continue to improve it as priorities allow.

[peterflynn]

Lgtm

[JeffryBooher]

won't calc (100% - 34px); work?

[JeffryBooher]

or I wonder if you just don't need the quotes max-height: calc(100vh - 34px);

[redmunds]

See the discussions in #7693.

calc(100vh - 34px) doesn't work in LESS because it tries to calculate the value at compile time (not run time).

The quotes are supposed to tell LESS to pass it through. I also tried in the LESS 1.7 branch (#6730), but it didn't work there either.

[redmunds]

I found an article that said to use ~"calc(100vh - 34px)" instead.

But, my brackets repo is hosed so I can't try it at the moment :(

[JeffryBooher]

Yeah I figured the quotes part out but that's strange that it doesn't work pass through. I tried ~"100% - 34px" and that didn't work which surprised me.

One thing I notice is that if the window gets to a certain height, you can't scroll all the way to the bottom.

Another thing is that the height always extends to the bottom of the window. I tried various vh percentages and couldn't find anything that worked. 90vh seemed to work the best but still i won't then I figured out the quote problem.

If you change this to max-height: ~"90hv"; it works much better.

[JeffryBooher]

I'm thinking less is the problem. Can we just attach the property at startup?

What I'm seeing is that I think less is computing the height of the window at compile time so if you change the window height then you can't scroll to the bottom item if you make it shorter.

[JeffryBooher]

[JeffryBooher]

So that didn't fix it either.

If you resize brackets to its min-height (on windows anyway) and set the max-height: 77vh it will let you scroll to the bottom item of the stock edit menu. you can't do that with 90vh; Then again, this probably isn't a very common case and it does cause the menu to add a scrollbar when it there is enough room for the last item.

[redmunds]

@JeffryBooher ~"calc(100vh - 34px)" doesn't seem to work either.

[TomMalbran]

@redmunds As I mentioned in the other issue, calc(~"100vh - 34px)" doesn't work because there is a bug in Blink, where calc doesn't work with viewport units. But according to this issue it is fixed in Chrome 34: https://code.google.com/p/chromium/issues/detail?id=168840. I would test it again when we merge the next CEF, but it might be fixed until the next update.

[JeffryBooher]

@redmunds done with review. Let me know what you think about that last comment. If you think it's worth fixing or not.

[redmunds]

@JeffryBooher I can see entire menu using 90vh. Here's my test:

1. Hack Global.js to force HTML Menus on Win7
2. Install String Convert extension which adds a dozen or so items to Edit menu
3. Reduce size of Brackets app vertically until Edit menu is forced to scroll
4. Invoke Edit menu
5. Use scrollbar to access last menu item

Note: Resizing Brackets while menu is showing is not supported. Resize Brackets first, then invoke menu.

[JeffryBooher]

The image above is the stock edit. At a certain point resizing vertically I can no longer get to the last item. it may be related to the fact that the shortcuts start wrapping due to the scrollbar which could be a bug with chromium. I'll try the string convert extension and see how that works.

[JeffryBooher]

it's probably not a big deal. It might be easier just to change the min-height of the shell but this is mainly for Linux users. I'm not sure we have a height restriction on Linux so i just wanted to know if you're ok with this not fixing every case.

[redmunds]

This solution does not fix every case, but is a simple fix for the common case.

We've been talking about whether or not to enforce a min height/width for a long time, but never decide on anything. There may even be an open PR for it.

[JeffryBooher]

Changes look good. merging