Inverting Flatlist does not render properly nor does scroll behavior work as expected

xboxdavchi commented 4 years ago

"react-native@0.60.6" "react-native-windows@0.60.0-vnext.133", node -v: 10.15.0 npm -v: 6.4.1

Target Platform Version(s): 10.0.18362.0
Target Device(s): Desktop
Visual Studio Version: VS 2017
Build Configuration: Debug & Release

Steps to Reproduce

Create a component that has a FlatList in it.
Set the inverted property to true

Expected Behavior

Assuming a list of items 0-100, the FlatList with inverted set to true should render with item 0 at the bottom of the list, and item 100 at the top of the list.

If you were to scroll up, the list should scroll up, and if you were to scroll down, the list should scroll down.

Actual Behavior

While the flat list does indeed have the items in the correct order, the whole flat list is flipped on its top edge, meaning that it renders entirely outside of its normal bounds.

The scroll behavior is also inverted. If you were to scroll up, the list scrolls down, and if you were to scroll down, the list scrolls up.

harinikmsft commented 4 years ago

Assigning to @chrisglein to investigate and figure out the right person to fix this.

rectified95 commented 4 years ago

Per https://reactnative.dev/docs/flatlist#inverted - the expected behaviour is that the direction of the scroll is reversed. Also, what do you mean be the list "renders entirely outside of its normal bounds"? The flatlist example in rntester in the Playground app renders correctly - perhaps you could check it out.

xboxdavchi commented 4 years ago

That is correct. But it also states how "Uses scale transforms of -1." meaning the list is now also flipped in addition to the direction of the scroll reverse. If you scroll to the top of the react native spec in the example and try out the inverted prop, you see the following:

EXPECTED Normal

Inverted:

ACTUAL Normal:

Inverted:

Where the red box is defining the normal bounds of the component.

I will investigate the playground app and report what I find there...

rectified95 commented 4 years ago

Thanks for replying. Here's a screenshot from my PC for reference:

xboxdavchi commented 4 years ago

As an additional clarification, I re-verified on the react native spec that the scroll behavior is not reversed-- moving the scroll bar up makes the scroll list move up. To clarify, this scroll behavior is observed when you use a mouse scroll wheel or keyboard-- not when you drag the scroll bar up and down.

The order in which the items is displayed is reversed though (as expected).

Update: I got the playground working, and I can verify the scroll behavior of flatlist is indeed different from the react native spec samples. So again, at the very least that part does not match the expected behavior. (still following up on the out-of-bounds rendering)

rectified95 commented 4 years ago

Confirmed, I got the same behaviour as you playing with the React spec editor-demoer. Will investigate. Let me know if you can repro the rendering misplacement issue.

rectified95 commented 4 years ago

@xboxdavchi Can you confirm that this issue is only about the inverted scroll and that the out-of-bounds rendering problem is longer there?

xboxdavchi commented 4 years ago

The out-of-bounds rendering problem still exists. I am not sure how the playground app gets around this, but when I create a flatlist with the inverted property set to true, the behavior is still as I described in my original set of diagrams.

I will see if I can create a sample app isolating this problem and send a screen cap of it for you.

xboxdavchi commented 4 years ago

Here is the UI with a flat list.

Here is the UI with the same flat list, but with the inverted flag set to true:

Notice how the flat list is now rendering out of its normal bounds (it should be at the bottom half of the screen, not flipped over the horizontal axis). Additionally, for some reason the element "aaa" is now missing from the list as well.

This is what I'm expecting:

Here is the code snippet for reference: <View style={{ flex: 1, justifyContent: "center", alignItems: "center", flexDirection: "column" }}> <View style={{ height: 500 }}> <BodyText style={{ color: Colors.White }}>{"flat list test"}</BodyText> </View> <FlatList style={{ alignSelf: "center", height: 200, width: 200, borderColor: Colors.White, backgroundColor: Colors.Red }} inverted={false} data={myData} renderItem={this.renderItem}> </FlatList> </View >

rectified95 commented 4 years ago

We believe that the rendering error has been fixed in #4169. Can you upgrade your version of RN Windows or cherry-pick it onto your source version and see if it persists?

As for the scroll direction part, there doesn't seem to be an easy solution to it now. Would you still consider it necessary, if the component rendered properly within its slot?

EDIT: @xboxdavchi I've created a blank app with the CLI, and can confirm that the list renders within its slot when inverted.

xboxdavchi commented 4 years ago

Thanks rectified95!

Unfortunately, the scroll direction part is still necessary for us, as we have a long list of items for this component despite the component rendering properly within its slot.

We will get back to you asap to confirm that the proposed fix resolves the rendering out of bounds issue.

rectified95 commented 4 years ago

@xboxdavchi Do you have updates on the rendering part of the problem?

xboxdavchi commented 4 years ago

Hey rectified95,

The issue we are running into now is trying to cherry pick the fix into our branch. We are effectively on 0.60, and the fix in 0.62 diverges enough that the cherry pick is non-trivial.

We will get back to you as soon as possible once we have figured out how to properly ingest the fix.

rectified95 commented 4 years ago

@xboxdavchi Do you have updates on your progress?

davidpchi commented 4 years ago

Hey Rectified95!

No new progress here, other than we are trying to figure out how to update now into 0.62 from our current fork.

In the interim, once we finally consume the fix, we'll need the proper scroll behavior regardless, so please keep us posted on that as well! I apologize for the delayed integration!

rectified95 commented 4 years ago

@davidpchi It looks like the fix on our end will require reimplementing the inversion - as long as the scaleY=-1 transform is passed to XAML under the hood, the scrolling will be incorrect. We're investigating how to model the desired behavior in a new way.

Can you tell me what your usage scenario is in more detail? For example:

whether it is about having a chat-like experience, where you will be adding new entries, and want them to appear at the bottom (would you want this addition to cause a slight scroll as well, to track the latest 'message'?)
will you be toggling the inverted property after your list has been rendered? If so, would you then want to see the first element rendered at the bottom, or would you be seeing the last element at the top, without the list still showing the first element, which is now at the bottom; this is assuming your list is bigger than the height of the control, and not all elements are visible.

NickGerleman commented 4 years ago

This doesn't seem within the scope of the initial 0.62 release. Flagging for retriage. We should backport this to 0.62 after release as its non-breaking and there's a customer need.

xboxdavchi commented 4 years ago

Hey Rectified95,

The usage scenario in this case is a chat-like experience (the first one you described). We will not toggle the inverted property after our list has been renedered.

chrisglein commented 4 years ago

This doesn't seem within the scope of the initial 0.62 release. Flagging for retriage. We should backport this to 0.62 after release as its non-breaking and there's a customer need.

It's a high profile partner bug. After many weeks of digging in, it looks like we need to re-architect how we implement inversion on FlatList, so I have low confidence this is trending towards the 0.62 release. But it is important.

Going to keep aspirationally (we would backport), but downgrading "must have".

chrisglein commented 4 years ago

If we need to flip the mouse wheel direction and the platform doesn't support this, then this may turn into a WinUI3 ask. @rectified95 let us know what recommendation you end up making here.

rectified95 commented 4 years ago

I'm investigating an alternative solution that would involve writing a custom panel for the XAML ScrollViewer we're using. Will report when I know more.

If we could just intercept the mouse delta, we wouldn't need to adjust a bunch of code in the JS implementation of the VirtualizedList, though.

rectified95 commented 4 years ago

This needs a change in XAML to work properly.

siddhant-rigi commented 9 months ago

Can't RN release an update to fix this issue forever.

microsoft / react-native-windows