feat(Custom Scrolling): Adds the ability to overwrite default grid scrolling #5841
Conversation
|
So this PR is taking over all scrolling of the grid. I wonder if we should make this a "toggle-able" feature? Perhaps, if you have pinned columns, this defaults to "ON", but if not, just use the native scrolling? Any known performance issues? Can this address the issues we've seen with horizontal scrollbar? The issue we have there is that we don't know when scrolling is happening (horizontally) so we have to always "show" the horizontal scrollbar. With this, perhaps we can fix the horizontal scrollbar issue with pinned columns since we will know when scrolling is occurring? |
| * @param {function} scrollHandler Function that needs to be called when scrolling happens. | ||
| */ | ||
| function gridScrolling(element, scrollHandler) { | ||
| var wrapper = element, pointX, pointY, startTime, startX, startY, maxScroll, |
There was a problem hiding this comment.
why rename element to wrapper?
There was a problem hiding this comment.
That is a good question. I didn't catch that when I was refactoring it. Will change it.
| wrapper.on('touchmove', move); | ||
| wrapper.on('touchcancel', end); | ||
| wrapper.on('touchend', end); | ||
| document.addEventListener('touchmove', function(e) { |
There was a problem hiding this comment.
this is turning off the ability to drag-scroll?
There was a problem hiding this comment.
I think so. I am actually not 100% certain on this one since I didn't touch this particular line.
| function start(event) { | ||
| var point = event.touches ? event.touches[0] : event; | ||
|
|
||
| wrapper.off('scroll', scrollHandler); |
There was a problem hiding this comment.
to understand this correctly, you are turning off native scrolling here and doing it yourself in this service?
There was a problem hiding this comment.
That is what I understand as well. So I believe you are correct.
|
@dlgski I believe I can make this a toggle-able feature, but I don't think it is a good idea to make it toggle based on the pinned attribute. Toggling it based on the pinned attribute would require making changing events on the go as pinned gets toggled and right now I believe all the events are initialized right when the grid gets initialized, so to support that we would need to update that logic even further. There are no known performance issues, in fact it seems to perform better in terms of keeping all scrolling in sync. And this can potentially help with the horizontal scroll bar issue, but that will require some further rework, so I would rather wait for a later version to figure that out. |
…ll event. Creating a wrapper service that takes over the default scrolling logic in order to ensure that grid scrolling works well in devices. Also, adds the ability to the grid to allow users to provide their own custom scrolling.
mportuga
changed the title
Creating a wrapper service that takes over the default scrolling logic in order to ensure that grid scrolling works consistently in both the browser and devices, as well as slow machines.
#1689, #3719, #5833