Desktop PWAs: App menu disappears before other icons when window too narrow |
||
Issue description
Chrome Version: 69
OS: Win10
When a PWA window is resized too narrow the app menu button is the first to go, it should be the last to go.
See screencast.
I've tried a quick fix by flipping the order of items in the HostedAppButtonContainer and doing an RTL flip during layout:
void HostedAppButtonContainer::Layout() {
views::View::Layout();
// Flip the order of child elements horizontally.
for (int i = 0; i < child_count(); ++i) {
views::View& child = *child_at(i);
if (child.visible())
child.SetX(width() - child.x() - child.width());
}
}
However this changes the tab ordering and the inner containers would need this flip as well. It's not a great solution.
I think the correct solution would be to extend BoxLayout to support an RTL horizontal orientation such that items are culled from the left instead of the right when there isn't enough space.
,
Jul 17
After discussing this with calamity it seems like BoxLayout should already support this behaviour. Using MAIN_AXIS_ALIGNMENT_END should behave like CSS' "justify-content: flex-end". Example: http://jsbin.com/sewutowowu/edit?css,output
,
Jul 18
Validating existing uses of MAIN_AXIS_ALIGNMENT_END to make sure they're compatible with this change. https://cs.chromium.org/chromium/src/ash/frame/caption_buttons/frame_caption_button_container_view.cc?type=cs&sq=package:chromium&g=0&l=190 This is okay. The titlebar will ensure there's enough space, even if there weren't it's better to ensure the close button is visible over the minimise button. https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc?l=1041 Clipping from the left would be the correct behaviour here, this usage is improved. https://cs.chromium.org/chromium/src/ui/message_center/views/notification_view_md.cc?l=1111 https://cs.chromium.org/chromium/src/ash/system/unified/unified_message_center_view.cc?l=75 https://cs.chromium.org/chromium/src/chrome/browser/ui/views/crostini/crostini_installer_view.cc?gsn=MAIN_AXIS_ALIGNMENT_END&g=0&l=343 https://cs.chromium.org/chromium/src/ash/login/ui/login_expanded_public_account_view.cc?l=120 These uses only have a single child, there will be no change in behaviour. https://cs.chromium.org/chromium/src/ash/login/ui/lock_contents_view.cc?l=287 https://cs.chromium.org/chromium/src/ash/message_center/message_center_view.cc?l=81 These have the entire screen real estate available to them, they will not overflow.
,
Jul 19
WIP CL https://chromium-review.googlesource.com/c/chromium/src/+/1143115 screencast.
,
Jul 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7aad75c0070a6d498059edd173e5f668053ce53 commit d7aad75c0070a6d498059edd173e5f668053ce53 Author: Alan Cutter <alancutter@chromium.org> Date: Tue Jul 24 09:06:36 2018 Make BoxLayout main axis clipping relative to main axis alignment This CL updates BoxLayout to avoid clipping the main axis alignment point when there isn't enough main axis space. This means for: - left aligned layouts the right most children will be clipped first (existing behaviour). - center aligned layouts the left and right most children will be clipped first. - right aligned layouts the left most children will be clipped first. Where previously the right most children were always clipped first regardless of the main axis alignment position. This fixes hosted app titlebar icons such that they clip from the left instead of clipping out the app menu button first. Analysis of existing alignment use cases here: https://bugs.chromium.org/p/chromium/issues/detail?id=862045#c3 Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=347613&signed_aid=Nst_-68AwDajlir_GX00cQ==&inline=1 After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=349265&signed_aid=RgUQgRr8Udaf6_WsZG31ig==&inline=1 Bug: 862045 Change-Id: I073cadced4d9b9172f9c803506c2faadd11683ca Reviewed-on: https://chromium-review.googlesource.com/1143115 Commit-Queue: Alan Cutter <alancutter@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#577463} [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/chrome/browser/ui/views/frame/glass_browser_frame_view_browsertest_win.cc [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/chrome/browser/ui/views/frame/hosted_app_button_container.cc [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/chrome/browser/ui/views/page_action/page_action_icon_container_view.cc [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/ui/views/layout/box_layout.cc [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/ui/views/layout/box_layout.h [modify] https://crrev.com/d7aad75c0070a6d498059edd173e5f668053ce53/ui/views/layout/box_layout_unittest.cc
,
Aug 1
|
||
►
Sign in to add a comment |
||
Comment 1 by alancutter@chromium.org
, Jul 10