Skip to content

feat(filter): add filterContainer option to allow a column filter to be placed in column menu #6901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 19, 2018
Merged

feat(filter): add filterContainer option to allow a column filter to be placed in column menu #6901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 44 additions & 31 deletions src/js/core/directives/ui-grid-header-cell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
(function() {
'use strict';

angular.module('ui.grid').directive('uiGridHeaderCell', ['$compile', '$timeout', '$window', '$document', 'gridUtil', 'uiGridConstants', 'ScrollEvent', 'i18nService',
function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, ScrollEvent, i18nService) {
angular.module('ui.grid').directive('uiGridHeaderCell', ['$compile', '$timeout', '$window', '$document', 'gridUtil', 'uiGridConstants', 'ScrollEvent', 'i18nService', '$rootScope',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
angular.module('ui.grid').directive('uiGridHeaderCell', ['$compile', '$timeout', '$window', '$document', 'gridUtil', 'uiGridConstants', 'ScrollEvent', 'i18nService', '$rootScope',
angular.module('ui.grid').directive('uiGridHeaderCell', ['$compile', '$timeout', '$window', '$document', '$rootScope', 'gridUtil', 'uiGridConstants', 'ScrollEvent', 'i18nService',

function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, ScrollEvent, i18nService, $rootScope) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move rootScope to the start? It is more of a personal preference, but I like having all of the $ variables grouped together.

Suggested change
function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, ScrollEvent, i18nService, $rootScope) {
function ($compile, $timeout, $window, $document, $rootScope, gridUtil, uiGridConstants, ScrollEvent, i18nService) {

// Do stuff after mouse has been down this many ms on the header cell
var mousedownTimeout = 500,
changeModeTimeout = 500; // length of time between a touch event and a mouse event being recognised again, and vice versa
Expand Down Expand Up @@ -226,6 +226,35 @@
}
};

var setFilter = function (updateFilters) {
if ( updateFilters ) {
if ( typeof($scope.col.updateFilters) !== 'undefined' ) {
$scope.col.updateFilters($scope.col.filterable);
}

// if column is filterable add a filter watcher
if ($scope.col.filterable) {
$scope.col.filters.forEach( function(filter, i) {
filterDeregisters.push($scope.$watch('col.filters[' + i + '].term', function(n, o) {
if (n !== o) {
uiGridCtrl.grid.api.core.raise.filterChanged();
uiGridCtrl.grid.api.core.notifyDataChange( uiGridConstants.dataChange.COLUMN );
uiGridCtrl.grid.queueGridRefresh();
}
}));
});
$scope.$on('$destroy', function() {
filterDeregisters.forEach( function(filterDeregister) {
filterDeregister();
});
});
} else {
filterDeregisters.forEach( function(filterDeregister) {
filterDeregister();
});
}
}
};

var updateHeaderOptions = function() {
var contents = $elm;
Expand Down Expand Up @@ -254,36 +283,12 @@
$scope.sortable = Boolean($scope.col.enableSorting);

// Figure out whether this column is filterable or not
var oldFilterable = $scope.filterable;
$scope.filterable = Boolean(uiGridCtrl.grid.options.enableFiltering && $scope.col.enableFiltering);

if ( oldFilterable !== $scope.filterable) {
if ( typeof($scope.col.updateFilters) !== 'undefined' ) {
$scope.col.updateFilters($scope.filterable);
}
var oldFilterable = $scope.col.filterable;
$scope.col.filterable = Boolean(uiGridCtrl.grid.options.enableFiltering && $scope.col.enableFiltering);

// if column is filterable add a filter watcher
if ($scope.filterable) {
$scope.col.filters.forEach( function(filter, i) {
filterDeregisters.push($scope.$watch('col.filters[' + i + '].term', function(n, o) {
if (n !== o) {
uiGridCtrl.grid.api.core.raise.filterChanged();
uiGridCtrl.grid.api.core.notifyDataChange( uiGridConstants.dataChange.COLUMN );
uiGridCtrl.grid.queueGridRefresh();
}
}));
});
$scope.$on('$destroy', function() {
filterDeregisters.forEach( function(filterDeregister) {
filterDeregister();
});
});
} else {
filterDeregisters.forEach( function(filterDeregister) {
filterDeregister();
});
}
}
$scope.$applyAsync(function () {
setFilter(oldFilterable !== $scope.col.filterable);
});

// figure out whether we support column menus
$scope.colMenu = ($scope.col.grid.options && $scope.col.grid.options.enableColumnMenus !== false &&
Expand Down Expand Up @@ -328,6 +333,14 @@

updateHeaderOptions();

if ($scope.col.filterContainer === 'columnMenu' && $scope.col.filterable) {
$rootScope.$on('menu-shown', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is $rootScope needed here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it might make sense to have tests that specifically change that setFilter works as expected when menu-shown event is triggered,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'menu-shown' event is emitted from the uiGridMenu directive, which is outside the uiGridHeaderCell directive, so $rootScope is needed to listen for the event from within uiGridHeaderCell. I'm open to suggestions if you think there might be a better way. We could perhaps move the filter stuff to another directive ('uiGrid'?).

$scope.$applyAsync(function () {
setFilter($scope.col.filterable);
});
});
}

// Register a data change watch that would get triggered whenever someone edits a cell or modifies column defs
var dataChangeDereg = $scope.grid.registerDataChangeCallback( updateHeaderOptions, [uiGridConstants.dataChange.COLUMN]);

Expand Down
12 changes: 10 additions & 2 deletions src/js/core/directives/ui-grid-menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18
scope: {
// shown: '&',
menuItems: '=',
autoHide: '=?'
autoHide: '=?',
col: '=?'
},
require: '?^uiGrid',
templateUrl: 'ui-grid/uiGridMenu',
Expand Down Expand Up @@ -157,8 +158,15 @@ function ($compile, $timeout, $window, $document, gridUtil, uiGridConstants, i18


// *** Auto hide when click elsewhere ******
var applyHideMenu = function() {
var applyHideMenu = function(event) {
if ($scope.shown) {
if ($scope.col && $scope.col.filterContainer === 'columnMenu') {
var elm = document.querySelector('.ui-grid-column-menu').querySelector('[ui-grid-filter]');
if (elm && elm.contains(event.target)) {
return false;
}
}

$scope.$apply(function () {
$scope.hideMenu();
});
Expand Down
3 changes: 3 additions & 0 deletions src/js/core/factories/GridColumn.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,9 @@ angular.module('ui.grid')
// Turn on filtering by default (it's disabled by default at the Grid level)
self.enableFiltering = typeof(colDef.enableFiltering) !== 'undefined' ? colDef.enableFiltering : true;

// Place the filter in the header cell by default
self.filterContainer = typeof(colDef.filterContainer) !== 'undefined' ? colDef.filterContainer : self.grid.options.filterContainer;

// self.menuItems = colDef.menuItems;
self.setPropertyOrDefault(colDef, 'menuItems', []);

Expand Down
11 changes: 11 additions & 0 deletions src/js/core/factories/GridOptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,17 @@ angular.module('ui.grid')
*/
baseOptions.enableFiltering = baseOptions.enableFiltering === true;

/**
* @ngdoc string
* @name filterContainer
* @propertyOf ui.grid.class:GridOptions
* @description Sets the parent element for the column filter. `headerCell` places
* it in the header cell. `columnMenu` places it in the column menu.
* Can be changed for individual columns using the columnDefs.
* Defaults to `headerCell`
*/
baseOptions.filterContainer = typeof(baseOptions.filterContainer) !== "undefined" ? baseOptions.filterContainer : "headerCell";

/**
* @ngdoc boolean
* @name enableColumnMenus
Expand Down
2 changes: 1 addition & 1 deletion src/templates/ui-grid/uiGridColumnMenu.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="ui-grid-column-menu">
<div ui-grid-menu menu-items="menuItems">
<div ui-grid-menu menu-items="menuItems" col="col">
<!-- <div class="ui-grid-column-menu">
<div class="inner" ng-show="menuShown">
<ul>
Expand Down
2 changes: 1 addition & 1 deletion src/templates/ui-grid/uiGridHeaderCell.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@
</i>
</div>

<div ui-grid-filter></div>
<div ui-grid-filter ng-if="col.filterContainer === 'headerCell'"></div>
</div>
3 changes: 3 additions & 0 deletions src/templates/ui-grid/uiGridMenu.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
leave-open="item.leaveOpen"
screen-reader-only="item.screenReaderOnly">
</li>
<li ng-if="col.filterable && col.filterContainer === 'columnMenu'">
<div ui-grid-filter></div>
</li>
</ul>
</div>
</div>
Expand Down
5 changes: 5 additions & 0 deletions test/unit/core/factories/GridOptions.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('GridOptions factory', function () {
scrollDebounce: 300,
enableSorting: true,
enableFiltering: false,
filterContainer: 'headerCell',
enableColumnMenus: true,
enableVerticalScrollbar: 1,
enableHorizontalScrollbar: 1,
Expand Down Expand Up @@ -85,6 +86,7 @@ describe('GridOptions factory', function () {
wheelScrollThrottle: 75,
enableSorting: true,
enableFiltering: true,
filterContainer: 'columnMenu',
enableColumnMenus: true,
enableVerticalScrollbar: 1,
enableHorizontalScrollbar: 1,
Expand Down Expand Up @@ -128,6 +130,7 @@ describe('GridOptions factory', function () {
scrollDebounce: 300,
enableSorting: true,
enableFiltering: true,
filterContainer: 'columnMenu',
enableColumnMenus: true,
enableVerticalScrollbar: 1,
enableHorizontalScrollbar: 1,
Expand Down Expand Up @@ -173,6 +176,7 @@ describe('GridOptions factory', function () {
aggregationCalcThrottle: 1000,
wheelScrollThrottle: 75,
enableFiltering: false,
filterContainer: 'columnMenu',
enableSorting: false,
enableColumnMenus: false,
enableVerticalScrollbar: 0,
Expand Down Expand Up @@ -216,6 +220,7 @@ describe('GridOptions factory', function () {
scrollDebounce: 300,
enableSorting: false,
enableFiltering: false,
filterContainer: 'columnMenu',
enableColumnMenus: false,
enableVerticalScrollbar: 0,
enableHorizontalScrollbar: 0,
Expand Down