Skip to content

Commit

Permalink
fix(menus): Fix menu positioning/animation
Browse files Browse the repository at this point in the history
Since v3.0.0-rc.21, the column/grid menu animation slides down from
above the grid, but it is visible the entire time. This causes the
animation to appear clunky. This has been resolved by setting `overflow:
hidden` on the menu.

Fixes angular-ui#3436, angular-ui#3921, angular-ui#3978.

Showing/hiding the menu can seem slow. The menu animation has been sped
up as well, and when it is hidden, the animation is now twice as fast.

Previously, to calculate the menu position, the width of the menu needed
to be calculated, which meant calling `repositionMenu` twice - once when
first opening the menu, and again after the menu is visible to allow the
width of the menu to be determined. This also meant there was always a
"shift" when the menu was first opened (as a default width of 170px was
used until the actual width of the menu could be determined. This
implements the CSS trick `right: 100%` so the width is not needed when
determining the menu's position, meaning the menu can be opened much
more smoothly and without needing to call `repositionMenu` twice.

Fixes angular-ui#6587.

remove lastMenuWidth from tests
  • Loading branch information
caseyjhol committed Mar 3, 2018
1 parent bf0267c commit 399784b
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 28 deletions.
15 changes: 4 additions & 11 deletions src/js/core/directives/ui-grid-column-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,17 +274,11 @@ function ( i18nService, uiGridConstants, gridUtil ) {

var containerScrollLeft = renderContainerElm.querySelectorAll('.ui-grid-viewport')[0].scrollLeft;

// default value the last width for _this_ column, otherwise last width for _any_ column, otherwise default to 170
var myWidth = column.lastMenuWidth ? column.lastMenuWidth : ( $scope.lastMenuWidth ? $scope.lastMenuWidth : 170);
var paddingRight = column.lastMenuPaddingRight ? column.lastMenuPaddingRight : ( $scope.lastMenuPaddingRight ? $scope.lastMenuPaddingRight : 10);

if ( menu.length !== 0 ){
var mid = menu[0].querySelectorAll('.ui-grid-menu-mid');
if ( mid.length !== 0 && !angular.element(mid).hasClass('ng-hide') ) {
myWidth = gridUtil.elementWidth(menu, true);
$scope.lastMenuWidth = myWidth;
column.lastMenuWidth = myWidth;

if ( mid.length !== 0 ) {
// TODO(c0bra): use padding-left/padding-right based on document direction (ltr/rtl), place menu on proper side
// Get the column menu right padding
paddingRight = parseInt(gridUtil.getStyles(angular.element(menu)[0])['paddingRight'], 10);
Expand All @@ -293,7 +287,7 @@ function ( i18nService, uiGridConstants, gridUtil ) {
}
}

var left = positionData.left + renderContainerOffset - containerScrollLeft + positionData.parentLeft + positionData.width - myWidth + paddingRight;
var left = positionData.left + renderContainerOffset - containerScrollLeft + positionData.parentLeft + positionData.width + paddingRight;
if (left < positionData.offset){
left = positionData.offset;
}
Expand Down Expand Up @@ -409,12 +403,11 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen

$scope.$on('menu-shown', function() {
$timeout( function() {
uiGridColumnMenuService.repositionMenu( $scope, $scope.col, $scope.colElementPosition, $elm, $scope.colElement );
//Focus on the first item
//automatically set the focus to the first button element in the now open menu.
gridUtil.focus.bySelector($document, '.ui-grid-menu-items .ui-grid-menu-item', true);
delete $scope.colElementPosition;
delete $scope.columnElement;
}, 200);
});
});


Expand Down
2 changes: 0 additions & 2 deletions src/js/core/directives/ui-grid-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
$elm.on('keyup', checkKeyUp);
$elm.on('keydown', checkKeyDown);
});
//automatically set the focus to the first button element in the now open menu.
gridUtil.focus.bySelector($elm, 'button[type=button]', true);
};


Expand Down
18 changes: 14 additions & 4 deletions src/less/header.less
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@

/* Slide up/down animations */
.ui-grid-column-menu .ui-grid-menu .ui-grid-menu-mid {
&.ng-hide-add, &.ng-hide-remove {
.transition(all, 0.05s, linear);
&.ng-hide-add {
.transition(all, 0.04s, linear);
display: block !important;
}

&.ng-hide-remove {
.transition(all, 0.02s, linear);
display: block !important;
}

Expand All @@ -148,8 +153,13 @@

/* Slide up/down animations */
.ui-grid-menu-button .ui-grid-menu .ui-grid-menu-mid {
&.ng-hide-add, &.ng-hide-remove {
.transition(all, 0.05s, linear);
&.ng-hide-add {
.transition(all, 0.04s, linear);
display: block !important;
}

&.ng-hide-remove {
.transition(all, 0.02s, linear);
display: block !important;
}

Expand Down
2 changes: 2 additions & 0 deletions src/less/menu.less
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
}

.ui-grid-menu {
overflow: hidden;
max-width: 320px;
z-index: 2; // So it shows up over grid canvas
position: absolute;
right: 100%;
padding: 0 10px 20px 10px;
cursor: pointer;
box-sizing: border-box;
Expand Down
5 changes: 3 additions & 2 deletions src/templates/ui-grid/uiGridMenu.html
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
<div
class="ui-grid-menu"
ng-if="shown">
ng-show="shown">
<style ui-grid-style>
{{dynamicStyles}}
</style>
<div
class="ui-grid-menu-mid"
ng-show="shownMid">
<div
class="ui-grid-menu-inner">
class="ui-grid-menu-inner"
ng-if="shown">
<ul
role="menu"
class="ui-grid-menu-items">
Expand Down
14 changes: 5 additions & 9 deletions test/unit/core/directives/ui-grid-column-menu.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,27 +461,25 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
renderContainerElm.querySelectorAll.calls.reset();
$elm.css.calls.reset();
});
describe('when the current column has a lastMenuWidth and lastMenuPaddingRight defined', function() {
describe('when the current column has a lastMenuPaddingRight defined', function() {
beforeEach(function() {
column = {
lastMenuWidth: 100,
lastMenuPaddingRight: 150
};
left = positionData.left + renderContainerElm.getBoundingClientRect().left - $scope.grid.element[0].getBoundingClientRect().left -
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width - column.lastMenuWidth +
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width +
column.lastMenuPaddingRight;
});
it('should use them to calculate the left position of the element', function() {
uiGridColumnMenuService.repositionMenu($scope, column, positionData, $elm, $columnElement);
expect($elm.css).toHaveBeenCalledWith('left', left + 'px');
});
});
describe('when the current column does not have lastMenuWidth and lastMenuPaddingRight defined, but $scope does', function() {
describe('when the current column does not have lastMenuPaddingRight defined, but $scope does', function() {
beforeEach(function() {
$scope.lastMenuWidth = 100;
$scope.lastMenuPaddingRight = 150;
left = positionData.left + renderContainerElm.getBoundingClientRect().left - $scope.grid.element[0].getBoundingClientRect().left -
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width - $scope.lastMenuWidth +
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width +
$scope.lastMenuPaddingRight;
});
it('should use them to calculate the left position of the element', function() {
Expand All @@ -491,10 +489,9 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
});
describe('when the left position is less the postion data offset', function() {
beforeEach(function() {
$scope.lastMenuWidth = 100;
$scope.lastMenuPaddingRight = 150;
left = positionData.left + renderContainerElm.getBoundingClientRect().left - $scope.grid.element[0].getBoundingClientRect().left -
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width - $scope.lastMenuWidth +
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width +
$scope.lastMenuPaddingRight;
positionData.offset = left + 10;
});
Expand All @@ -508,7 +505,6 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
$elm[0].querySelectorAll.and.returnValue([{
querySelectorAll: jasmine.createSpy('querySelectorAll').and.returnValue('<div></div>')
}]);
$scope.lastMenuWidth = 100;
$scope.lastMenuPaddingRight = 150;
left = positionData.left + renderContainerElm.getBoundingClientRect().left - $scope.grid.element[0].getBoundingClientRect().left -
renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width - gridUtil.elementWidth() +
Expand Down

0 comments on commit 399784b

Please sign in to comment.