fix(menus): streamline menu positioning

Menu width calculation was removed in #6588. Menu width needs to be calculated in order to properly position the menu (if there is not enough room to the left of the column). repositionMenu is now always called in the 'menu-shown' event listener to ensure the width can be properly calculated. The style attribute is removed from $elm in the 'menu-hidden' event listener to prevent the menu from appearing to slide from the left or right when opening another column menu while one is already open. Menu animation speed was changed in #6588 to have different add/remove speeds. 0.04s has been chosen as a happy medium, and the $timeout duration has been changed to reflect the new transition duration (to reduce delay when hiding the menu). `position: relative` has been removed from .ui-grid-header-cell-row to ensure consistent calculation of offsetParent across browsers. Fixes #5396, #5990, #6085.
angular-ui · Mar 11, 2018 · a83df5b · a83df5b
1 parent f14da2f
commit a83df5b
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 20 deletions.
diff --git a/src/js/core/directives/ui-grid-column-menu.js b/src/js/core/directives/ui-grid-column-menu.js
@@ -274,6 +274,9 @@ function ( i18nService, uiGridConstants, gridUtil ) {
 
       var containerScrollLeft = renderContainerElm.querySelectorAll('.ui-grid-viewport')[0].scrollLeft;
 
+      // repositionMenu is now always called after it's visible in the DOM,
+      // allowing us to simply get the width every time the menu is opened
+      var myWidth = gridUtil.elementWidth(menu, true);
       var paddingRight = column.lastMenuPaddingRight ? column.lastMenuPaddingRight : ( $scope.lastMenuPaddingRight ? $scope.lastMenuPaddingRight : 10);
 
       if ( menu.length !== 0 ){
@@ -288,8 +291,9 @@ function ( i18nService, uiGridConstants, gridUtil ) {
       }
 
       var left = positionData.left + renderContainerOffset - containerScrollLeft + positionData.parentLeft + positionData.width + paddingRight;
-      if (left < positionData.offset){
-        left = positionData.offset;
+
+      if (left < positionData.offset + myWidth) {
+        left = Math.max(positionData.left - containerScrollLeft + positionData.parentLeft - paddingRight + myWidth, positionData.offset + myWidth);
       }
 
       $elm.css('left', left + 'px');
@@ -356,7 +360,6 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
           $scope.$broadcast('hide-menu', { originalEvent: event });
         } else {
           $scope.menuShown = true;
-          uiGridColumnMenuService.repositionMenu( $scope, column, colElementPosition, $elm, $columnElement );
 
           $scope.colElement = $columnElement;
           $scope.colElementPosition = colElementPosition;
@@ -384,10 +387,11 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
 
 
       $scope.$on('menu-hidden', function() {
+        $elm[0].removeAttribute('style');
+
         if ( $scope.hideThenShow ){
           delete $scope.hideThenShow;
 
-          uiGridColumnMenuService.repositionMenu( $scope, $scope.col, $scope.colElementPosition, $elm, $scope.colElement );
           $scope.$broadcast('show-menu');
 
           $scope.menuShown = true;
@@ -403,6 +407,8 @@ function ($timeout, gridUtil, uiGridConstants, uiGridColumnMenuService, $documen
 
       $scope.$on('menu-shown', function() {
         $timeout( function() {
+          uiGridColumnMenuService.repositionMenu( $scope, $scope.col, $scope.colElementPosition, $elm, $scope.colElement );
+
           //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;

diff --git a/src/js/core/directives/ui-grid-menu.js b/src/js/core/directives/ui-grid-menu.js
@@ -139,7 +139,7 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
               $scope.shown = false;
               $scope.$emit('menu-hidden');
             }
-          }, 200);
+          }, 40);
         }
 
         angular.element(document).off('click touchstart', applyHideMenu);

diff --git a/src/less/header.less b/src/less/header.less
@@ -53,7 +53,6 @@
 
 .ui-grid-header-cell-row {
   display: table-row;
-  position: relative
 }
 
 .ui-grid-header-cell {
@@ -130,16 +129,11 @@
 
 /* Slide up/down animations */
 .ui-grid-column-menu .ui-grid-menu .ui-grid-menu-mid {
-  &.ng-hide-add {
+  &.ng-hide-add, &.ng-hide-remove {
     .transition(all, 0.04s, linear);
     display: block !important;
   }
 
-  &.ng-hide-remove {
-    .transition(all, 0.02s, linear);
-    display: block !important;
-  }
-
   &.ng-hide-add.ng-hide-add-active,
   &.ng-hide-remove {
     .transform(translateY(-100%));
@@ -153,16 +147,11 @@
 
 /* Slide up/down animations */
 .ui-grid-menu-button .ui-grid-menu .ui-grid-menu-mid {
-  &.ng-hide-add {
+  &.ng-hide-add, &.ng-hide-remove {
     .transition(all, 0.04s, linear);
     display: block !important;
   }
 
-  &.ng-hide-remove {
-    .transition(all, 0.02s, linear);
-    display: block !important;
-  }
-
   &.ng-hide-add.ng-hide-add-active,
   &.ng-hide-remove {
     .transform(translateY(-100%));

diff --git a/test/unit/core/directives/ui-grid-column-menu.spec.js b/test/unit/core/directives/ui-grid-column-menu.spec.js
@@ -422,7 +422,7 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
 		});
 
 		describe('repositionMenu: ', function() {
-			var column, left, positionData, renderContainerElm, $elm, $columnElement;
+			var column, left, positionData, renderContainerElm, $elm, $columnElement, myWidth;
 
 			beforeEach(function() {
 				column = {};
@@ -452,6 +452,8 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
 				spyOn(gridUtil, 'closestElm').and.returnValue(renderContainerElm);
 				spyOn(gridUtil, 'elementWidth').and.returnValue(100);
 				spyOn(gridUtil, 'getStyles').and.returnValue({paddingRight: 30});
+
+				myWidth = gridUtil.elementWidth();
 			});
 			afterEach(function() {
 				gridUtil.closestElm.calls.reset();
@@ -497,7 +499,7 @@ describe('ui-grid-column-menu uiGridColumnMenuService', function() {
 				});
 				it('should use the position data offset to calculate the left position of the element', function() {
 					uiGridColumnMenuService.repositionMenu($scope, column, positionData, $elm, $columnElement);
-					expect($elm.css).toHaveBeenCalledWith('left', positionData.offset + 'px');
+					expect($elm.css).toHaveBeenCalledWith('left', positionData.offset + myWidth + 'px');
 				});
 			});
 			describe('when ui-grid-menu-mid is defined and visible', function() {