schrodinger / fixed-data-table-2

A React table component designed to allow presenting millions of rows of data.
http://schrodinger.github.io/fixed-data-table-2/
Other
1.29k stars 289 forks source link

Scroll wheel affects table even when not focused #316

Closed quisido closed 6 years ago

quisido commented 6 years ago

Expected Behavior

My scroll wheel should be scrolling the element my mouse is on top of.

Current Behavior

If the fixed data table is on the page, the scroll wheel scrolls the fixed data table instead of any other component.

Possible Solution

Don't.

Context

This may be due to the other scrollable component utilizing a custom scroll bar instead of an innate window scroll bar. However, if the fixed data table is the only element on the screen, and you move your mouse outside of it and scroll the scroll wheel, the fixed data table will scroll. This is not intended behavior, and I believe it to be the root of the issue. I can find no documentation for turning this behavior off.

Your Environment

Chrome Windows desktop

tl;dr - if my mouse is not on top of the data table, I do not want the scroll wheel to scroll the data table. It seems to be using some sort of window-level event listener.

wcjordan commented 6 years ago

I'm not able to reproduce this on the examples (using a Linux machine though). Does this occur for you on the examples?

quisido commented 6 years ago

I guess it does depend on the machine. I am on my home station now, and cannot reproduce it (albeit wit ha trackpad instead of scroll wheel). My work station does, however. Do you have a link to the examples page? I can test each on Monday.

wcjordan commented 6 years ago

The docs & examples can be found here: http://schrodinger.github.io/fixed-data-table-2/

quisido commented 6 years ago

I've found it's actually just a conflict with Material UI's components. If a Modal element exists simultaneously, the fixed-data-table already receives scroll focus from anywhere on the page. I don't know why yet, but I'd say it's a problem with their repo, not this one, so I'm closing.

wcjordan commented 6 years ago

Sounds good. Let me know if there's anything further I can help with.

quisido commented 6 years ago

After some serious debugging, I believe I've found the issue, so I'll re-open for discussion.

fixed-data-table-2 has an onWheel event listener for scrolling. Said event listener called event.preventDefault, which prevents other elements from having their scroll event listeners from firing.

So you have fixed-data-table-2 in the background and a <div> with a scroll event listener floating above the data table in the foreground. With your mouse focusing the foreground div, move the mouse wheel. The wheel event will not find an event listener on the div and proceed to propagate through other elements until it finds one. It finds one on data table. Data table scrolls and prevents default. The events stop, and div never fires its scroll event listener. This is particularly obscure, because we are using custom scrollbars, making it so that the custom scrollbar doesn't actually scroll until it receives a scroll event, which it never does because of this.

I have been doing tons of brainstorming on this issue to figure out how to best resolve it, and figured I'd throw it out here as well.

One solution is to add a wheel listener to the floating div. This does resolve the problem. By having the wheel listener on the div call the scroll listener and stopPropagation, the data table never receives the event and does not scroll.

However, this does not match what one would expect. An element that renders scrolling behavior in response to a scroll event listener should fire said listener without the need of a mouse wheel listener in addition. I imagine that data table uses a mouse listener because it is not actually scrolling but rendering a scrollbar to mimic the appearance of scrolling, whereas my custom scrollbars are actually scrolling an overflowing element.

The optimal solution, I think, is for the data table not to respond to mouse wheel events unless the event target is not listening to scroll events. (If the event target is listening to wheel events, it can stop propagation itself, if desired.)

I'm not sure how easily this is implementable, but as I type this I wonder how possible the solution would be to simply check the event target. Should data table respond to mouse wheel events if it is not the event target?

Something along the lines of:

<div ref={(div) => { this.rootRef = div; }}>
  <FixedDataTableJSX />
</div>
onWheel = (e) => {
  let target = e.target;
  while (target !== this.rootRef) {
    if (target === document.body) {
      return true;
    }
    target = target.parentNode;
  }
  // scroll listener here
}

This will thusly only scroll data table is the element the mouse is over is a part of data table. If the element the mouse is over is any other element, data table will not scroll.

This is pretty open ended as to which solution is best. Should floating elements be required to have wheel listeners in order to prevent Data Table from intercepting the event? Or should Data Table's wheel listener not interrupt target elements' scroll events?

wcjordan commented 6 years ago

I think I understand now, although I'd make one clarifying ask since 1 of 2 things may be happening. 1) The floating panel is a child of table and the table is receiving the scroll event through the DOM scroll bubbling. 2) The floating panel has something like pointer-events disabled so mouse events go to the element behind it instead of itself.

For the rest of this answer, I'll assume case 1 since that's the one I see more frequently. If this is the case, it's very similar to #242 and has a sample solution, which is in line with the hacky solution you mention in your description. The alternative solution which would be cleaner would be to change the DOM layout so the modal is not a child of the table or not use a React portal so the event doesn't bubble. One way to do this is to create a React component which is mounted by the top component, but only shows when some global / redux state is set. Then the table can set that redux / global state whenever we want to show the component.

quisido commented 6 years ago

The floating panel is a child of table and the table is receiving the scroll event through the DOM scroll bubbling.

The floating panel is mounted within the child of table as a React element, however in the DOM, the floating panel is appended to the end of <body> (not even within the #root element that React is mounted in). In particular, this floating element is a material-ui/Popover component, which renders a material-ui/Modal component.

The floating panel has something like pointer-events disabled so mouse events go to the element behind it instead of itself.

I'm not familiar with what this is, and digging into its code, I saw nothing similar to this? It uses theDiv.addEventListener('scroll', this.handleScroll). This method never fires. However, disabling event.preventDefault() in Data Table's wheel event does allow handleScroll to fire.

If I add a wheel event to the floating panel, and tell it to stopPropagation, Data Table's wheel event does not fire if the mouse if placed atop the floating panel. So I believe it accurate to say that the floating panel, when moused over, does accurately receive priority of events. However, the scroll event is never reached due to the Data Table's wheel event listener.

The solution for #242 does look similar to what solved my problem (the custom scrollbar being given a wheel event listener that stops propagation), but I don't think the problem is with React. I don't think Data Table should be intercepting wheel events when it is not the event.target.

wcjordan commented 6 years ago

Here's a React issue which describes some more details of what you're seeing: https://github.com/facebook/react/issues/11387

The gist is that React's event bubbling treats your modal as a child of the table, similar to how the DOM would bubble events from a button up to the containing div.

quisido commented 6 years ago

But is there anything wrong with it doing that? I still feel as if Data Table should not be responding to wheel events for which it (and its children) are not the target. Unless React is doing some odd faux event triggering and telling Data Table that it is the target, I am assuming event.target is still the floating panel, even when Data Table receives the event.

I can't think of a scenario where Data Table should be responding to wheel events for elements outside of its DOM children, despite React triggering those events.

wcjordan commented 6 years ago

You're right, except since React uses it's own event system, not the DOM event system, it bubbles based on the child components. To clear this up, I think the best way forward is to move the modal so it's not a child of the data table. I'm happy to look at PRs, but I'll want to do some perf investigation before walking DOM trees as part of the wheel handler so we don't cause scroll lag.

quisido commented 6 years ago

I think both are valid alterations. My panel should not be rendering inside Data Table and bubbling its events to Data Table, but also Data Table should not be responding to events for which it is not the target. Ideally, both solutions would be implemented, imo, as I think both are technically correct implementations with or without conflict.

The lag inspection sounds like a reasonable investigation. Do you want a PR on v1.0-beta branch or master branch?

wcjordan commented 6 years ago

I'm fine with either. Whichever you're working off of works.

quisido commented 6 years ago

I opened a PR for master, albeit a novice attempt at open source contribution.

wcjordan commented 6 years ago

now fixed w/ v0.8.13