Skip to content

Commit

Permalink
fix(cellNav): Don't setup when directive not there
Browse files Browse the repository at this point in the history
The cellNav feature was attaching logic to render containers and grid
cells despite the uiGridCellnav directive not being on the parent grid
component. This was causing exception to be thrown during scroll events.

This change looks for the cellNav controller (which is simply
empty) and bails before attaching logic if the controller is not present.

Also added tests to cover the exceptions that were occuring.

Fixes #2128
  • Loading branch information
c0bra committed Dec 2, 2014
1 parent b0e36aa commit 9bfa6e3
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
21 changes: 15 additions & 6 deletions src/features/cellnav/js/cellnav.js
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@
priority: -150,
require: '^uiGrid',
scope: false,
controller: function () {},
compile: function () {
return {
pre: function ($scope, $elm, $attrs, uiGridCtrl) {
Expand Down Expand Up @@ -696,15 +697,17 @@
return {
replace: true,
priority: -99999, //this needs to run very last
require: ['^uiGrid', 'uiGridRenderContainer'],
require: ['^uiGrid', 'uiGridRenderContainer', '?^uiGridCellnav'],
scope: false,
compile: function () {
return {
pre: function ($scope, $elm, $attrs, uiGridCtrl) {
},
post: function ($scope, $elm, $attrs, controllers) {
var uiGridCtrl = controllers[0],
renderContainerCtrl = controllers[1];
renderContainerCtrl = controllers[1],
cellNavController = controllers[2];

// Skip attaching cell-nav specific logic if the directive is not attached above us
if (!cellNavController) { return; }

var containerId = renderContainerCtrl.containerId;

Expand Down Expand Up @@ -765,9 +768,15 @@
return {
priority: -150, // run after default uiGridCell directive and ui.grid.edit uiGridCell
restrict: 'A',
require: '^uiGrid',
require: ['^uiGrid', '?^uiGridCellnav'],
scope: false,
link: function ($scope, $elm, $attrs, uiGridCtrl) {
link: function ($scope, $elm, $attrs, controllers) {
var uiGridCtrl = controllers[0],
cellNavController = controllers[1];

// Skip attaching cell-nav specific logic if the directive is not attached above us
if (!cellNavController) { return; }

if (!$scope.col.colDef.allowCellFocus) {
return;
}
Expand Down
26 changes: 26 additions & 0 deletions src/features/cellnav/test/uiGridCellNavDirective.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
describe('ui.grid.cellNav directive', function () {
var $scope, $compile, elm, uiGridConstants;

beforeEach(module('ui.grid.cellNav'));

beforeEach(inject(function (_$rootScope_, _$compile_, _uiGridConstants_) {
$scope = _$rootScope_;
$compile = _$compile_;
uiGridConstants = _uiGridConstants_;

$scope.gridOpts = {
data: [{ name: 'Bob' }]
};
}));

it('should not throw exceptions when scrolling when a grid does NOT have the ui-grid-cellNav directive', function () {
elm = angular.element('<div ui-grid="gridOpts"></div>');

$compile(elm)($scope);
$scope.$digest();

expect(function () {
$scope.$broadcast(uiGridConstants.events.GRID_SCROLL, {});
}).not.toThrow();
});
});

0 comments on commit 9bfa6e3

Please sign in to comment.