fix(menus): Fix menu positioning/animation

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 #3436, #3921, #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 #6587.

remove lastMenuWidth from tests

no message

Author: caseyjhol

src/js/core/directives/ui-grid-column-menu.js

@@ -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);
@@ -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;
       }
@@ -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);
+        });
       });
 
 

src/js/core/directives/ui-grid-menu.js

@@ -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);
       };
 
 

src/less/header.less

@@ -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;
   }
 
@@ -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;
   }
 

src/less/menu.less

@@ -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;

src/templates/ui-grid/uiGridMenu.html

@@ -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">

test/unit/core/directives/ui-grid-column-menu.spec.js

@@ -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() {
@@ -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;
 				});
@@ -508,13 +505,12 @@ 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() +
+						renderContainerElm.querySelectorAll()[0].scrollLeft + positionData.parentLeft + positionData.width +
 						gridUtil.getStyles().paddingRight;
 				});
-				it('should use the position menu width and right padding to calculate the left position of the element', function() {
+				it('should use the position and right padding to calculate the left position of the element', function() {
 					uiGridColumnMenuService.repositionMenu($scope, column, positionData, $elm, $columnElement);
 					expect($elm.css).toHaveBeenCalledWith('left', left + 'px');
 				});