Commit 51d855cd authored by Alex Danilo's avatar Alex Danilo Committed by Commit Bot

Fix regression in sub-menu resize

Sub-menus are reduced in height when the viewport height
is reduced so the sub-menu won't fit without clipping,
however it isn't increasing height when viewport height
is increased. This CL fixes it, by always setting the
sub menu height and toggling the overflowY behaviour
(auto or scroll).

Bug: 934207
Tests: Ran all browser and unit tests, added unit test.
Change-Id: I33ec8720b8350924b8e631debede8cad4eb5fb05
Reviewed-on: https://chromium-review.googlesource.com/c/1481285Reviewed-by: default avatarLuciano Pacheco <lucmult@chromium.org>
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634544}
parent 424c0887
......@@ -130,9 +130,13 @@ cr.define('cr.ui', function() {
style.top = itemRect.top + 'px';
// Size the subMenu to fit inside the height of the viewport
const menuEndGap = 18; // padding on cr.menu + 2px
// Always set the maximum height so that expanding the window
// allows the menu height to grow crbug/934207
style.maxHeight = (viewportHeight - itemRect.top - menuEndGap) + 'px';
if ((itemRect.top + childRect.height + menuEndGap) > viewportHeight) {
style.maxHeight = (viewportHeight - itemRect.top - menuEndGap) + 'px';
style.overflowY = 'scroll';
} else {
style.overflowY = 'auto';
}
},
......
......@@ -20,11 +20,13 @@ function setUp() {
'<style>',
' cr-menu {',
' position: fixed;',
' padding: 8px;',
' }',
' cr-menu-item {',
' width: 10px;',
' height: 10px;',
' display: block;',
' background-color: blue;',
' }',
'</style>',
'<command id="default-task">',
......@@ -40,6 +42,7 @@ function setUp() {
'</cr-menu>',
'<cr-menu id="sub-menu" hidden>',
' <cr-menu-item class="custom-appearance"></cr-menu-item>',
' <cr-menu-item class="custom-appearance"></cr-menu-item>',
'</cr-menu>',
].join('');
......@@ -190,3 +193,43 @@ function testClickOutsideVisibleMenuAndSubMenu() {
assertTrue(topMenu.hasAttribute('hidden'));
assertTrue(subMenu.hasAttribute('hidden'));
}
/**
* Tests that shrinking the window height will limit
* the height of the sub-menu.
*/
function testShrinkWindowSizesSubMenu() {
testSelectHostMenuItemAndCallShowSubMenu();
const subMenuPosition = subMenu.getBoundingClientRect();
// Reduce window innerHeight so sub-menu won't fit.
window.innerHeight = subMenuPosition.bottom - 10;
// Call the internal hide method, then re-show it
// to force the resizing behavior.
menubutton.hideSubMenu_();
menubutton.showSubMenu();
const shrunkPosition = subMenu.getBoundingClientRect();
assertTrue(shrunkPosition.bottom < window.innerHeight);
}
/**
* Tests that growing the window height will increase
* the height of the sub-menu.
*/
function testGrowWindowSizesSubMenu() {
// Remember the full size of the sub-menu
testSelectHostMenuItemAndCallShowSubMenu();
const subMenuPosition = subMenu.getBoundingClientRect();
// Make sure the sub-menu has been reduced in height.
testShrinkWindowSizesSubMenu();
// Make the window taller than the sub-menu plus padding.
window.innerHeight = subMenuPosition.bottom + 20;
// Call the internal hide method, then re-show it
// to force the resizing behavior.
menubutton.hideSubMenu_();
menubutton.showSubMenu();
const grownPosition = subMenu.getBoundingClientRect();
// Test that the height of the sub-menu is the same as
// the height at the start of this test (before we
// deliberately shrank it).
assertTrue(grownPosition.bottom === subMenuPosition.bottom);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment