RTL support is slightly broken for Safari and Edge

[bvaughn]

Based on the Mozilla docs it looks like Chrome is reports incorrect values for scrollLeft when the direction is "rtl":

If the element's direction is rtl (right-to-left), then scrollLeft is 0 when the scrollbar is at its rightmost position (at the start of the scrolled content), and then increasingly negative as you scroll towards the end of the content.

Rather than reporting a range of <= 0, Chrome reports the distance in pixels measured from the left. (View the console output in this repro case.)

I've tried to accommodate this broken behavior (fb15909 and dcb9c55) but the fix is kludgy.

It also doesn't fully work for Safari and Edge:

1. Go to the demo page
2. Scroll left a bit
3. Scroll back right

It looks like when the scroll position reaches the right, the scrollLeft value sometimes rolls over from 0 to a really high number, causing the list to blank out.

I am not very familiar with RTL layouts. Help would be appreciated.

[davidgarsan]

At least in the case of Safari, the scrollLeft high value seems because of the bounce effect when reaching the limit of scroll in a RTL scenario.

[davidgarsan]

A working progress branch is on going.

[MarkFalconbridge]

I think #198 is as a result of the manipulation of scrollLeft to calculate the offset for this issue. The onScroll event emitted by the grid has the manipulated offset value rather than the scrollLeft position that the browser being used understands. If it were possible to keep the normalisation of the offset internal to react-window #198 would be at least partially fixed. Would it be useful to use a library such as https://github.com/alitaheri/normalize-scroll-left to work around these issues?

[MarkFalconbridge]

I've attempted to fix the problems caused by the varying ways browsers report scrollLeft in rtl in this branch.

Hopefully it fixes the issues seen in this bug as well as #198. Before I write unit tests to try to test what I've written I'd like some feedback as to whether it's even likely to get accepted as a pull request @bvaughn .

What it's doing is maintaining a normalized version of the scrollLeft value internal to itself and leaving the scrollLeft version emitted in onScroll events be that of the browser. Similarly the scrollTo method assumes that it's going to receive scrollLeft values appropriate for the browser being used and then calculates the normalized value appropriately. scrollToItem was more difficult, it has to work out where to scroll the browser to once more items have been rendered.

I've modified the ScrollToItem website file to be able to drive the interactions in rtl.

[MarkFalconbridge]

I'm actually not sure how to meaningfully test the fixes in this branch without writing tests based around intern or something similar because the issue is specifically related to browser implementations of scrollLeft when the direction is rtl. Using the existing jest framework wouldn't really prove that the fix works because jest uses jsdom so I'd end up writing tests that drive the code in a very specific way based around the way the fixes have been written. It would be much better to have functional tests that interact with a real browser. Any thoughts @bvaughn ?

[davidgarsan]

@MarkFalconbridge as you can see in my comment above, I was working in a fix for this issue.
To the date I was able to fix it in the List component, but the Grid is still pending.
The PR was tested ok in Chrome, Firefox and Safari.
Unfortunately, I've not been able to work in that since then, but definitely I'll continue that soon.
Did you try the branch?

[MarkFalconbridge]

@davidgarsan I did try your branch but we need to use the list and the grid and both the scrollTo and scrollToItem methods and have it work in Chrome, Firefox and Edge. Because we're writing code that uses it now we couldn't really wait.

[davidgarsan]

I understand. Currently I only need the List, but soon I'll work with the Grid so I need to have it fixed by then.
Unfortunately I won't be able to work in this in the next 2 or 3 weeks.

[bvaughn]

Fixed via #251

[bvaughn]

Bug fix released in 1.8.2 - https://github.com/bvaughn/react-window/blob/master/CHANGELOG.md#182

[MarkFalconbridge]

Looks good but I'm still seeing an issue in Edge. Go to https://react-window.now.sh/#/examples/list/fixed-size-rtl using Edge and click the scroll to 500 buttons. The content of the list/grid disappears. Using the horizontal scroll bars results in the grid/list content appearing/disappearing.

[bvaughn]

Works for me, but I'm testing with OSX Edge preview.

[MarkFalconbridge]

I'm using Microsoft Edge 44.17763.1.0 (Microsoft EdgeHTML 18.17763) and it fails on that.

[MarkFalconbridge]

As far as I can see your fix means that when used in RTL anyone using the onScroll methods can only use the scrollLeft value contained in them for driving another react-window component because the scrollLeft values represent react-window's normalized version of the browser's scrollLeft value. That's fine but should probably be documented in the descriptions of onScroll.