Horizontal header scrolling broken with autoHeight=true

[timclutton]

When the option autoHeight=true is set headers do not scroll horizontally when the grid content is scrolled via the scrollbars.

This can be seen in example 11 - No vertical scrolling (autoHeight). Here's a GIF showing the behaviour (note I edited the CSS to shorten the height):

Scrolling via the mousewheel does work (because that doesn't call handleScroll()). In the GIF you can see the headers snap to the correct position when scrolling vertically.

This appears to be caused by the fix to #674, which assumes no scroll handling is required when autoHeight=true.

The fix for #674 should be reverted and the following change made in _handleScroll() instead:

- if (vScrollDist) {
+ if (vScrollDist && !options.autoHeight) {

This solves this issue and #674 remains fixed as well.

[ghiscoding]

it looks like it's @6pac that made this commit causing the issue, I'm not sure of the implication but Ben wrote this.

Thanks for the bug report. The solution was to prevent the grid handling the scroll event when autoheight is being used.

slick.grid.js:

function handleScroll() {
  + // autoheight does not have scrolling, but editors can trigger a scroll, which we should ignore
  + if (options.autoHeight) return;
  
  scrollTop = $viewportScrollContainerY[0].scrollTop;
  scrollLeft = $viewportScrollContainerX[0].scrollLeft;
  return _handleScroll(false);
}

this will be included in the next release.

@6pac Ben do you have more info on this and can we somehow fix the other issue in a way that doesn't regress?

[6pac]

eeep. I clearly didn't think of that use case when I made the patch! Thanks @timclutton will apply your fix as suggested.
@ghiscoding I belive this will fix the original issue correctly.

[6pac]

@timclutton I've patched as suggested.
The original fix, as the code comment says, was because we can have an editor that allows H or V scrolling, and these scrolling events are captured by handleScroll(). I'm just thinking about what will happen with the same type of editor in the scenario where there is no horizonal scrollbar.

[6pac]

OK, have played with the test case a bit. It's not scrolling within the editor, it's the fact that the editor can add a div that expands the canvas and makes a scrollbar appear in the autoHeight mode (which will disappear when the editor is closed). So we just don't handle scrolling in the grid in that case. I think we're good.

[timclutton]

Thanks @6pac for the quick fix. Minor feedback would be to have the comment state autoheight does not have *vertical* scrolling, but that doesn't merit another commit.

[ghiscoding]

also please note that we are now on version 3.x and most probably won't push fixes on 2.x versions anymore. Basically, this fix will only be available in 3.x versions

[6pac]

good documentation always justifies another commit!

[6pac]

@ghiscoding I think we should keep a 2.x branch patched with bug fixes at least for a time, perhaps 12 months, I'm happy to do the work. I just think people need some time to change over to the new configuration. Would you like to set up a branch for that, or shall I?

[ghiscoding]

@6pac unfortunately, that's seems quite hard (not without a lotttt of work), the reason is because we didn't keep a branch of the old version 2.x. The only thing that you could do is to create a new version 2 branch and reapply all previous code, but again that seems quite hard since the last tag in 2.x is 2.4.44, however I know that your last version 2.4.45 so on top of reapplying previous code (1 file at a time), you would have to also reapply anything between version 2.4.44 and 2.4.45. If you have time to kill and want to do all that work, go ahead, but I'm telling you that's really a lot of work... Or wait, maybe another thing you could do is to get these files from unpkg - slickgrid, again that would require you to create a new v2 branch and overwrite every files 1 file at a time... If you have time to kill, that would be the steps, have fun 😉

[ghiscoding]

@timclutton the fix was released under latest version 3.0.2

[6pac]

@timclutton There is a new repo https://github.com/6pac/Slickgrid-2.x that will be maintained for a while for those who aren't ready to do the whole update of the background files. I'll apply ongoing non-3.0 related fixes there for as long as it's not too difficult.