angular-ui / ui-grid

UI Grid: an Angular Data Grid
http://ui-grid.info
MIT License
5.39k stars 2.47k forks source link

ng-grid 2.0.14, ngAnimate and the new Google Chrome update: selected row is not highlighted #3574

Closed germanger closed 8 years ago

germanger commented 9 years ago

I'm using the last 2.x ng-grid, that is: 2.0.14. But maybe someone can give me a hint with this issue.

Google Chrome has updated to 43.0.2357.65 m and I've noticed that because of that, now my ng-grids are failing to represent a selected row.

This is the symptom:

Scenario 1:

  1. I click on a row
  2. The item represented by that row is now in the array of selected items, but the row is not highlighted

Scenario 2.

  1. The ng-grid lives under a bootstrap-ui tab
  2. I click on a row
  3. The item represented by that row is now in the array of selected items, but the row is not highlighted
  4. I click on a different bootstrap-ui tab (that tab content is shown)
  5. I click back on the tab where the ng-grid lives (the original tab content is shown)
  6. Now the row is highlighted (as it should had been in step 3)

I havent touched my code and I'm 100% sure that this is because of the new Google Chrome update.

Also when I go to the ng-grid demo page (http://angular-ui.github.io/ng-grid/), when I click a row, the selected row is indeed highlighted... so this probably has something to do with how I'm using ng-grid. Here is how I use it. Althugh I used the same settings in one of the plunkers and the row selection was fine...

$scope.receptionsGrid = { 
    data: 'receptions',
    selectedItems: $scope.selectedReceptions,
    rowHeight: 24,
    enableHighlighting: true,
    enablePinning: false,
    enableColumnResize: true,
    useExternalSorting: true,
    sortInfo: { fields: [], columns: [], directions: [] },
    enableSorting: false,
    keepLastSelected: false,
    columnDefs: [
        {field:'receptionId', displayName:'ID DB', cellClass:'align-right', width: 50},
        {field:'exporterId', displayName:'exp', cellClass:'align-right', width: 50},
        {field:'season.code', displayName:'season', cellClass:'align-right', width: 50},
        {field:'code', displayName:'code', cellClass:'align-right', width: 65},
        {field:'date', displayName:'date', cellClass:'align-right', cellFilter:'date:\'yyyy-MM-dd\'', width: 80},
        {field:'grower.code', displayName:'prod', cellClass:'align-right', width: 60},
        {field:'dummy', displayName: ' '}],
    multiSelect: false,
    beforeSelectionChange: function () {
        return !($scope.fetchingLots);
    },
    afterSelectionChange: function (rowItem, event) {
        $scope.lots.length = 0;
        $scope.selectedLots.length = 0;

        if ($scope.selectedReceptions.length > 0) {
            $scope.fetchLots();                
        }
    }
};

Here is a video showing both scenarios described: https://www.youtube.com/watch?v=syEvhQN82Lc

I've also noticed that using the Chrome Inspector, after selecting or deselecting a row, the CSS is updated correctly in code (ie: the row gets the class .ngRow.selected if selected). But that class acquisition isn't reflected in the UI:

image

Any help? Thanks.

germanger commented 9 years ago

Okay I've managed to isolate the problem! The problem is the module angular-animate and the new version of Google Chrome

Check these two plunkers

Plunkr 1: App without ngAnimate http://plnkr.co/edit/2pSBX9K0QaeaSihMKnGG?p=preview When selecting a row, the row is highlighted

Plunkr 2: App WITH ngAnimate http://plnkr.co/edit/hyRO4fTwglSCL8KCTgHA?p=preview When selecting a row, the row isn't highlighted

Can someone help me fix this? I'm using last angular and angular-animate versions (v1.3.15)

germanger commented 9 years ago

I've created a 3rd plunkr:
http://plnkr.co/edit/cWMlKEz39n8K52VWH9q8?p=preview

This is a fork of the 2nd plunkr, in which I've disabled animations for every item that doesn't have the class "angular-animate" in it, ie:

app1.config(['$animateProvider', function($animateProvider){
   $animateProvider.classNameFilter(/angular-animate/);
}]);

This works (now rows are highlighted after selection) but if you are using animations in your app, this will mostly break every other animation! like bootstrap-ui modals for example. So, this is not a solution, but an idea: I need to disable animations for ng-grid only. How do I achieve that?

classNameFilter(x) enables animations for only the items with the class x in them. Is there a similar function for disabling animations for certain items? and in case there is: How do I disable animations for everything that has to do with ng-grid?

maksim-marholin commented 9 years ago

Bug confirmed - we have the same problem in our app after the .65 Chrome release. No other workarounds found, except some dirty jQuery (DOM) hooks...

germanger commented 9 years ago

If you are using bootstrap ui and few third party modules with animations, you will probably be fine with this:

  app1.config(['$animateProvider', function($animateProvider){
        $animateProvider.classNameFilter(/modal/);
 }]);

In my case, the only animated thing apart from modals are toasters (angular-toaster), so...

  app1.config(['$animateProvider', function($animateProvider){
        $animateProvider.classNameFilter(/modal|toast/);
 }]);
maksim-marholin commented 9 years ago

Thanks a lot, I saw this solution and I will probably use it, but I would still prefer to avoid such kind of 'hacks', if I have a choice. Thanks anyway!

germanger commented 9 years ago

But aren't worse those "dirty jQuery (DOM) hooks" you mentioned? How are you solving now the problem? I'm interested in that choice you mention

maksim-marholin commented 9 years ago

DOM hooks are definitely worse, you are absolutely right. I will use your solution for now, but we are really going to replace ng-grid completely because of some other weird bugs we've got using it. So not sure I will even try to search for a better solution and you can see that the 'choice' is not the one which works for everybody. The good thing here is that I see something that works in IE, but not in Chrome at last :)

PaulL1 commented 9 years ago

If you replace ng-grid completely.....logically ui-grid would be the replacement?

maksim-marholin commented 9 years ago

It's one of the options, but I have some concerns, that part of these bugs is moved into ui-grid. Both these grids are pretty good for Angular infrastructure, but not sure they will be suitable for our growing app (as Angular at all). We need to deal with 500k+ records data sets, so taking a look at clusterize.js and jqGrid (which in my opinion is one of the most mature, well-maintained and functional implementations, have free-version fork, but ugly design). Anyway, all this stuff is under strong consideration now.

germanger commented 9 years ago

With the new angular 1.4.0 (which has some breaking changes in ngAnimate) this problem is even worse....

PaulL1 commented 9 years ago

I'm not sure that ng-grid 2.x supports 1.3 or 1.4 of angular.

dustinschultz commented 9 years ago

Can also confirm for Version 43.0.2357.124 m. I noticed that the selection changed after re-sizing the window, which forces a grid refresh/redraw.

This workaround worked for me:

$scope.gridOptions = {
    afterSelectionChange: function(rowItem) {
        // select your row
        // ...
        //

        // Re-build columns, otherwise selection isn't highlighted
        scope.gridOptions.ngGrid.buildColumns();
    }
};
germanger commented 9 years ago

Thats a reasonable workaround when you have one grid, but in my case I have like 30 grids... so I would have to put that line in all afterSelectionChange's

Its too sad ng-grid 2.x was abandonned because of the so promised ng-grid 3. I was pretty happy with 2.x until this bug appeared.

I've seen ng-grid 3 specs and its full of things I wont ever use. ng-grid 2.x is simple and works (well... worked)

PaulL1 commented 9 years ago

2.x isn't abandoned as such. It's still a project that people can contribute to. The problem is that there are no devs left who appear willing to contribute to it. But at any time people can decide to do so, and it will be alive again.

As for 3.0 - perhaps try it before you write it off? It might turn out to suit you.

jliang6 commented 9 years ago

Try this:

afterSelectionChange: function(rowItem, event) {
        var x = document.querySelectorAll(".ng-scope .ngRow");
        x[rowItem.rowIndex].style["webkitUserSelect"] = "none";
        $timeout(function() {
            x[rowItem.rowIndex].style["webkitUserSelect"] = "text";
        }, 100);
}

This fix works in several projects. Remember to DI $timeout

germanger commented 9 years ago

Between that and @dustinschultz's solution:

afterSelectionChange: function(rowItem) {
    // Re-build columns, otherwise selection isn't highlighted
    scope.gridOptions.ngGrid.buildColumns();
}

I prefer @dustinschultz's.

Also I haven't tried this but I'm pretty sure it would work too:

afterSelectionChange: function(rowItem) {
    $(window).resize();
}

I've used the $(window).resize() before to fix ng-grids that are in hidden bootstrap-ui tabs

The problem with all these is that you have to modify every grid definition. Its a reasonable solution if you have 1 grid or few

choroshin commented 9 years ago

I created a simple plugin that you can register in ng-grid's options object :

//controller
$scope.gridOptions = { data : "myData",
   plugins: [ new ngGridFixChromeSelectionBugPlugin() ]
};

//plugin
function ngGridFixChromeSelectionBugPlugin() {
        var self = this;
        self.init = function (scope, grid, services) {
            self.services = services;
            self.grid = grid;
              //check if the browser is Chrome (for performance issues).
            if (navigator.userAgent.toLowerCase().indexOf('chrome') > -1) {
                // mousedown event on row selection
                grid.$viewport.on('mousedown', self.onRowMouseDown);
            }
        };
        self.onRowMouseDown = function (event) {
            // Get the closest row element from where we click.
            var targetRow = $(event.target).closest('.ngRow');          
            if (targetRow) {
                self.grid.buildColumns();
            }
        }; 
    }
pford68 commented 9 years ago

I can confirm this issue too after recently upgrading NPM modules. (I am using the Angular NPM modules because I am using Browserify.) The first fix above from germanger worked. I have not tried the second one. And the problem occurs only in Chrome.

So apparently we will need to switch to ui-grid going forward?

pford68 commented 9 years ago

@choroshin's solution looked the most elegant, but it did not work for me. @dustinschultz's and @jliang6's worked for me. @germanger's did too, but it looks scary.

On a side note, one wrinkle in my case is the selected row truly was highlighted as I scrolled horizontally. So the previously selected row would have some cells highlighted, and the newly selected row would have others highlighted.

By the way, I have a demo tomorrow. And I discovered this at the last minute. Ironically I was working on one particular view all last week, but that view just happens to have the only grid that does not exhibit this issue...so that's fun. I can implement a long-term solution, like moving to ui-grid, later.

PoonamSomani commented 9 years ago

Neither of scope.gridOptions.ngGrid.buildColumns(); or $(window).resize(); is working for me as suggested by @germanger . Any suggestions?