fix(Grid.js): ScrollIfNecessary does not account for scrollWidth correctly #6669
Conversation
…ectly Since scrollIfNecessary is called multiple times when enableCellEditOnFocus is true we need to make sure the scrollbarWidth and footerHeight is accounted for to not cause a loop. fixes angular-ui#6653
| | ||
| //Since scrollIfNecessary is called multiple times when enableCellEditOnFocus is true we need to make sure the scrollbarWidth and footerHeight is accounted for to not cause a loop. | ||
| if (gridCol.colDef.enableCellEditOnFocus === true) { | ||
| scrollPixels = scrollPixels - self.footerHeight - self.scrollbarWidth; |
There was a problem hiding this comment.
Is there a reason why we wouldn't want to check this every time? Why are we only checking for the footer and scroll bar when enableCellEditOnFocus is true?
There was a problem hiding this comment.
The downside that I have found in leaving it on for everything is that it will scroll a bit too much for the grids not using enableCellEditOnFocus. It makes navigating by keys not appear as smooth and that might make it confusing.
There was a problem hiding this comment.
Hmm, I see, makes sense. Also, as a follow up, could you add a null or undefined check for gridCol instead of assuming it will be there. I ask, because i noticed that a few lines now, we are checking for that and I don't want this to ever blow up on users of this function. I believe refactoring it to the following should work:
if (gridCol && gridCol.colDef && gridCol.colDef.enableCellEditOnFocus) {
There should be no need to explicitly compare enableCellEditOnFocus to true either.
After that, I will merge it and I will see if I release a new version at some point this coming week. I usually try to accumulate at leas 5 bug fixes before doing a new release.
Make sure gridCol is not null before checking for enableCellEditOnFocus
3 participants
Since scrollIfNecessary is called multiple times when enableCellEditOnFocus is true we need to make sure the scrollbarWidth and footerHeight is accounted for to not cause a loop.
fixes #6653