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 #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
  • Loading branch information
caseyjhol authored and mportuga committed Mar 5, 2018
1 parent 0e83225 commit 25dbd2e
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 30 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
18 changes: 7 additions & 11 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,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');
});
Expand Down

0 comments on commit 25dbd2e

Please sign in to comment.