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

Context menus & HTML main menus are too narrow, wrapping text #4593

Closed
julieyuan opened this issue Jul 29, 2013 · 19 comments
Closed

Context menus & HTML main menus are too narrow, wrapping text #4593

julieyuan opened this issue Jul 29, 2013 · 19 comments
Assignees
@julieyuan
Labels
IQE low priority
Milestone
Sprint 33

Comments

@julieyuan
Copy link

Steps:

  1. Launch Brackets.
  2. Open a html or css file.
  3. Right click on one line to bring up context menu.

Results:
The string "Quick Docs" and its shortcut key ⌘k is not in the same line.

Expected:
The layout will be better if they are in the same line.

ENV: MAC 10.8.3 French OS
Build: 0.28.0-8524

Notes: This issue not reproduces in Sprint27

Snapshots:
Please refer to snapshot for details:
screen shot 2013-07-28 at 11 54 24 pm

Sprint27 for reference:
screen shot 2013-07-28 at 11 54 44 pm
@peterflynn
Copy link
Member

This seems to be happening with the English working set context menu in Sprint 28 now. Nominating for Sprint 29 since I think that's a regression.

@ghost ghost assigned jasonsanjose Aug 5, 2013
@pthiess
Copy link
Contributor

pthiess commented Aug 8, 2013

Removing the milestone

@peterflynn peterflynn mentioned this issue Aug 23, 2013
Closed
@peterflynn
Copy link
Member

Updated title to reflect that this covers multiple menus

@peterflynn peterflynn mentioned this issue Oct 3, 2013
Closed
@peterflynn
Copy link
Member

Note: this also affects the HTML menu bar (visible on Linux or the in-browser branch).

@peterflynn
Copy link
Member

See PR #5398 for a potential solution.

@peterflynn
Copy link
Member

Removing 'French' & loc tags since this is now a much more general bug.

@ghost ghost assigned peterflynn Oct 3, 2013
@peterflynn
Copy link
Member

Taking this from @jasonsanjose as I'm trying to sort it out now (and/or #5398)

@peterflynn
Copy link
Member

Ok, so I think this is what's going on:

  • The menu items are a stack of <li>s with the shortcut set to float: right. This works fine in isolation.
  • The menu popup has position: absolute. Still works fine in isolation.
  • The menu bar entry is a parent of the menu popup, and it has position: relative. Disaster! 💥 Here's why...
  • The width of a pos: absolute item is just enough to fit its content, but with a max of its containing block's width (which in turn can be overridden by min-width constraints or "incompressible" content like <img>s, unbroken words, or no-wrap text).
  • So... in this case the containing block is the pos: relative menu bar item (it needs to be pos: rel to position the popup correctly horizontally, btw). But this containing block is typically only 40-50px wide (just enough for the label e.g. "File"; the popup menu doesn't prop open its width due to it being pos: abs, which is good because it would lead to lots of empty space between the menu bar items).
  • So the popup menu effectively has a max width of just 40-50px. But only sort of -- the menu item labels are no-wrap (probably because of this exact problem!), and Bootstrap also gives the popups a min-width: 160px. Both help make the menu not-entirely-ridiculously narrow. But it still wants to be as narrow as it can get without breaking those two constraints, so it wraps at every float opportunity it can find. 💥

Blergh. I can't think of any totally pretty options here. A couple suggestions to follow, though...

Also, regarding context menus: the problem is the same there, just less visually obvious. To keep Bootstrap happy we parent context menus to an empty "menu bar item" anchor, which since it has no label winds up being 0x0 -- even narrower than the menu bar case. (Making this invisible anchor 100% width is the fix in #5398, but obviously we can't do that in the menu bar because it will ruin the label spacing).

@peterflynn
Copy link
Member

Ok, so the options...

  1. Explicitly ask the browser to size the items to their "preferred" content width, overriding the implicit max from the pos: rel parent. We can do this via width: -webkit-max-content, but it won't work in IE or possibly Safari. So it's not ideal since we'd like to have an in-browser version of Brackets eventually.
  2. Get JS into the picture, either for manually sizing the menu items or (probably cleaner) manually positioning them. With manual positioning, we could make the menu bar items pos: static or maybe even parent the popups to <body> where they can't fall into such parent-size traps.
  3. Redo the menu item using table layout (display: table-cell, etc. of course). This isn't as crazy as it sounds -- we currently don't lay out our menus the way native apps do, and this would fix that. I.e. to truly have a unified left-hand boundary to the the "shortcut column" of the menu, it needs to be considered a unified "column" by the browser (rather than a stack of unrelated things containing floats, as today). Not that I think our current non-native layout looks bad... but ditching it would fix this bug!

@peterflynn
Copy link
Member

I think I'm going to have to start using ":boom:" icons as a "QED" marker more often when explaining bug mechanisms :-)

@redmunds
Copy link
Contributor

redmunds commented Oct 3, 2013

@peterflynn Have you tried whitespace: no-wrap; ? But, I'm not sure it will work with the right-aligned text.

@peterflynn
Copy link
Member

@redmunds The text is right-aligned using float, and afaik there's no way to make a float not wrap. The table layout change is the closest thing I can think of to that. (You could also use pos: abs and right: 0 to right-align, but that would yield an even worse layout, with text overlap).

peterflynn added a commit that referenced this issue Oct 3, 2013
@peterflynn
Fix bug #4593 (HTML menus / context menus are too narrow: keyboard sh…
fd933c7 
…ortcut

wraps to second line in some places) -- see notes in bug for the sordid
details...
@redmunds
Copy link
Contributor

redmunds commented Oct 9, 2013

@peterflynn Sounds like this used to work as we want it to. Did you use git bisect to see what commit changed this?

@peterflynn
Copy link
Member

Good point Randy! Looks like it broke sometime between Sprint 28 & Sprint 31, so it was fairly recent (notably, more recent than the switch to Bootstrap 2...). I'll give it a shot.

@peterflynn
Copy link
Member

Crud, getting different results in a dev environment though...

@peterflynn
Copy link
Member

Ok, so it seems like what changed is Chromium -- if I run our current master in the Sprint 28 shell, no bug / if I run the sprint-28 tagged code in our current shell, bug. Unfortunately, it seems like Chromium is being more correct now though...

peterflynn added a commit that referenced this issue Oct 19, 2013
@peterflynn
Fix bug #4593 (Wrap text in context menus & HTML main menus) -- see bug
eeefd68 
thread for all the gory details. This fix preserves the current layout
(where label & shortcut are not two strictly separated columns), but the
downside is it may not work in other browsers.

May want to revisit later for that reason, but in the meantime this will
improve the Linux build (which has no native menubar yet).
@peterflynn peterflynn mentioned this issue Oct 19, 2013
Merged
@marcelgerber marcelgerber mentioned this issue Oct 20, 2013
Closed
JeffryBooher added a commit that referenced this issue Oct 22, 2013
@JeffryBooher
Merge pull request #5580 from adobe/pflynn/html-menu-wrap
aa14bbb 
Fix bug #4593 (Text wrap in context menus & HTML main menus)
@peterflynn
Copy link
Member

Oops actually, wasn't filed by me -- so FBNC @julieyuan

@ghost ghost assigned julieyuan Oct 23, 2013
@peterflynn peterflynn mentioned this issue Oct 23, 2013
Closed
@julieyuan
Copy link
Author

Thanks for all your efforts. I will verify it once I get the latest build.

@julieyuan
Copy link
Author

Checked this issue and #4895 together with build 0.33.0-10152. It is fixed. So closing it.Here are snapshots for reference:
11
screen shot 2013-10-28 at 6 26 31 pm
screen shot 2013-10-28 at 6 26 09 pm

@core-ai-bot core-ai-bot mentioned this issue Aug 30, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@julieyuan julieyuan

Labels
IQE low priority
Projects
None yet
Milestone
Sprint 33
Development

No branches or pull requests
5 participants
@jasonsanjose @redmunds @peterflynn @pthiess @julieyuan