Browser zoom in / out does not work correctly when table has mouse context

[weaseliam]

Current Behavior

On Ctrl + Mouse Wheel Up / Down, the table scrolls up / down. The browser zooms in / out only when the table reaches on top / bottom.

Expected Behavior

On Ctrl + Mouse Wheel Up / Down, the browser should zoom in / out. The table should not scroll on Ctrl + Mouse Wheel Up / Down.

Possible Solution

I saw that in ReactWheelHandler.js::onWheel, an event.preventDefault() is called. When I commented it, the browser took into account the Ctrl + Mouse Wheel Up / Down. Of course when I changed the method to just return if Ctrl is pressed, otherwise leave the code as is, it worked as expected on all browsers.

Steps to Reproduce (for bugs)

• The table should have many results so that the scrollbar appears
• Scroll the table so that the scrollbar is at the half of it
• Have the mouse cursor over the table
• Press Ctrl + Mouse Wheel Up or Down
• The table scrolls then when it reaches the top or bottom, the browser zooms in / out

Your Environment

• Version used: 0.7.7, 0.7.10
• Browser Name and version: Chrome 55.0.2883.75, Firefox 49.0.2, IE 11
• Operating System and version (desktop or mobile): Windows 7

[KamranAsif]

I think the ReactWheelHandler fix is fine. Can you put up a PR?

[wcjordan]

Note if we modify ReactWheelHandler, we'll be forking from fbjs: https://github.com/facebook/fbjs/blob/e66ba20ad5be433eb54423f2b097d829324d9de6/packages/fbjs/src/dom/ReactWheelHandler.js

I'm not sure there's a better way though. One thine you could explore would be overriding both _shouldHandleWheelX and _shouldHandleWheelY to return false if CTRL is pressed. We need state for tracking if CTRL is pressed though, but I think any solution may require that (unless I'm wrong and it's available somewhere on the wheel event).

[KamranAsif]

@wcjordan MouseEvent has properties if alt/ctrl/shift/meta was held down when fired.

[wcjordan]

Nice, that makes things easier. Maybe we should just pass the event to _shouldHandleWheelX and _shouldHandleWheelY and have it return false if CTRL is pressed? Then we could submit a PR to fbjs to add the some functionality there since it wouldn't be a breaking change.

[weaseliam]

Indeed after having a deeper look into fixed-data-table-code, not just the surface, I agree that passing the event as the last argument in ReactWheelHandler to _handleScrollX and _handleScrollY and doing the fix in _shouldHandleWheelX / _shouldHandleWheelY seems the way to go. In fact I do not understand why didn't they pass it in the first place.

[KamranAsif]

I'm okay with either fix, since they both require a PR to the mainline. I don't think we are merging it in before 1.0 so feel free to put up a PR