Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTL Bug Android #544

Open
1 task
Stevemoretz opened this issue Aug 1, 2022 · 22 comments
Open
1 task

RTL Bug Android #544

Stevemoretz opened this issue Aug 1, 2022 · 22 comments
Labels
bug Something isn't working P1 Important but not urgent

Comments

@Stevemoretz
Copy link

Current behavior

On RTL Mode, the list is completely weird.

RTL:

SVID_20220801_164855.mp4

LTR:

SVID_20220801_164922.mp4

Expected behavior

Well it should simply behave like LTR but only the direction changes.

To Reproduce

I'm sorry I don't use anything but expo managed workflow so I couldn't get your sample app to work, but here's a simple repro.

import {FlashList} from "@shopify/flash-list";
import React, {Component} from "react";
import {FlatList, ScrollView, Text, View, I18nManager} from "react-native";

export default function App() {
    const data1 = [1, 2];
    const data2 = [
        1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
    ];

    React.useEffect(() => {
        I18nManager.forceRTL(true);
        I18nManager.allowRTL(true);
    }, []);

    return (
        <View>
            <View style={{marginTop: 100}} />
            <FlashList
                horizontal
                data={data2}
                estimatedItemSize={100}
                renderItem={(info) => {
                    return (
                        <View
                            style={{
                                marginHorizontal: 8,
                                width: 100,
                                height: 100,
                                backgroundColor: "gray",
                            }}
                        >
                            <Text>{info.item}</Text>
                        </View>
                    );
                }}
            />
        </View>
    );
}

Remember you need to restart your app to get the I18nManager's effect.

Platform:

  • iOS
  • [ X ] Android

Environment

@shopify/flash-list@1.2.0
npx expo-env-info

  expo-env-info 1.0.5 environment info:
    System:
      OS: macOS 10.15.7
      Shell: 5.7.1 - /bin/zsh
    Binaries:
      Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
      Yarn: 1.22.19 - ~/.nvm/versions/node/v16.16.0/bin/yarn
      npm: 8.15.1 - ~/.nvm/versions/node/v16.16.0/bin/npm
      Watchman: 2022.02.14.00 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 14.4, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
      Android SDK:
        API Levels: 28, 29, 30, 32
        Build Tools: 28.0.3, 29.0.2, 29.0.3
    IDEs:
      Android Studio: 4.0 AI-193.6911.18.40.6626763
      Xcode: 12.4/12D4e - /usr/bin/xcodebuild
    npmPackages:
      expo: ~45.0.0 => 45.0.6 
      react: 17.0.2 => 17.0.2 
      react-dom: 17.0.2 => 17.0.2 
      react-native: 0.68.2 => 0.68.2 
      react-native-web: 0.17.7 => 0.17.7 
      react-navigation: 4 => 4.4.4 
    npmGlobalPackages:
      eas-cli: 0.56.0
      expo-cli: 6.0.1
    Expo Workflow: managed

I though giving your library try because FlatList's RTL is corrupted on the latest version:
facebook/react-native#34314

Had no idea yours doesn't work too :(

It's a horrible time for having an RTL app right now!

@Stevemoretz Stevemoretz added the bug Something isn't working label Aug 1, 2022
@naqvitalha
Copy link
Collaborator

Have you looked into whether something like this works: facebook/react-native#19150 (comment)
If the RLV patch works we can apply the same in FlashList

@Stevemoretz
Copy link
Author

Have you looked into whether something like this works: facebook/react-native#19150 (comment) If the RLV patch works we can apply the same in FlashList

Thank you for the response that's a very good suggestion, I was waiting for him to confirm if it works if he doesn't by the end of today, I'll try it myself and report back here.

@Stevemoretz
Copy link
Author

Have you looked into whether something like this works: facebook/react-native#19150 (comment) If the RLV patch works we can apply the same in FlashList

Got some free time, I thought I'd give it a try now.

modified this file :
node_modules/recyclerlistview/dist/reactnative/core/VirtualRenderer.js according to that PR

    VirtualRenderer.prototype.updateOffset = function (offsetX, offsetY, isActual, correction) {
        if (this._viewabilityTracker) {
            console.log(offsetX,this.getLayoutDimension().width, offsetY, isActual, correction);
            var offset = this._params && this._params.isHorizontal ? I18nManager.isRTL && Platform.OS === "android" ?
                this.getLayoutDimension().width - offsetX :
                offsetX : offsetY;
            if (!this._isViewTrackerRunning) {
                if (isActual) {
                    this._viewabilityTracker.setActualOffset(offset);
                }
                this.startViewabilityTracker(correction);
            }
            this._viewabilityTracker.updateOffset(offset, isActual, correction);
        }
    };

It didn't solve the case that console.log that I provided might provide some useful info:

LTR Start of the List :

0 2320 0 true Object {
  "endCorrection": 0,
  "startCorrection": 0,
  "windowShift": -0,
}

LTR End of the List :

1960 2320 0 true Object {
  "endCorrection": 0,
  "startCorrection": 0,
  "windowShift": -0,
}

RTL Start of the List :

1640 2000 0 true Object {
  "endCorrection": 0,
  "startCorrection": 0,
  "windowShift": -0,
}

RTL End of the List :

0 2000 0 true Object {
  "endCorrection": 0,
  "startCorrection": 0,
  "windowShift": -0,
}

Sound like this.getLayoutDimension().width returns different results in RTL and LTR same for the offsetX.

@Stevemoretz
Copy link
Author

Stevemoretz commented Aug 3, 2022

I took a diff on layout manager in RTL and LTR modes

https://editor.mergely.com/IHHq4Qi9/

(Removed that PR patch as it was not helping thought)

@Stevemoretz
Copy link
Author

Gave it another shot without the margin so there would be no estimation involved here are the diff results.

https://editor.mergely.com/Dhvh0v7n/

nextIndex or isOverridden is the issue probably?
I can't find where nextIndex comes from though!!

@Stevemoretz
Copy link
Author

Found where nextIndex comes from :
node_modules/@shopify/flash-list/dist/utils/AverageWindow.js

Do you have some free time to take a look at this whole issue? I couldn't really get the pieces together sounds a little too complicated for me who have no background about how this library works and nor the other one.

Thanks, please let me know if you do or don't want to go through fixing this.

@Stevemoretz
Copy link
Author

Have you looked into whether something like this works: facebook/react-native#19150 (comment) If the RLV patch works we can apply the same in FlashList

According to facebook/react-native#19150 (comment)
He could make it work, I added pagingEnabled={true} and applied that PR again, but it doesn't work in FlashList sounds like this is an issue specifically about FlashList as the above information I provided also suggests the same thing, sounds like : (Based on the logs I provided above)

  1. Estimation in RTL mode doesn't work at all.
  2. That patch doesn't do anything special here.
  3. As the video shows some items are missing in the list, and there is some overlapping.
  4. If you remove margins so there would be no estimation the overlapping issue goes away now you get some items just not in the right order and some of them are invisible. what I get is this : 1, 14, empty, 12, empty, 10, multiple empty pieces, 18, 19, 20

@mmmoussa
Copy link
Contributor

mmmoussa commented Aug 7, 2022

Tested the RLV patch in my app using the latest version of flashlist and confirmed it doesn't fix the problem, which I mentioned in #520 (comment). I got sidetracked on other tasks but am trying to reserve some time to investigate this issue.

@Stevemoretz
Copy link
Author

Tested the RLV patch in my app using the latest version of flashlist and confirmed it doesn't fix the problem, which I mentioned in #520 (comment). I got sidetracked on other tasks but am trying to reserve some time to investigate this issue.

Thank you so much for reporting this back! Been stuck on this whole rtl list thing for more than a week now :(

@mmmoussa
Copy link
Contributor

mmmoussa commented Aug 7, 2022

Did some more testing and found that flashlist is not even working correctly for iOS RTL still. Will try to put up a stripped down example repo.

@Stevemoretz
Copy link
Author

Did some more testing and found that flashlist is not even working correctly for iOS RTL still. Will try to put up a stripped down example repo.

Did you get a chance to proceed on that? If I could do anything useful please let me know.
This rtl thing is really upsetting react native flatlist also has a bug in it which they don't even care to fix it.

@Stevemoretz
Copy link
Author

Did some more testing and found that flashlist is not even working correctly for iOS RTL still. Will try to put up a stripped down example repo.

Expo sdk 46 supports this so I made a minimal demo with its snack if its helps: https://snack.expo.dev/@stevemoretz/flashlist-rtl-bug

@id3vz
Copy link

id3vz commented Sep 9, 2022

@Stevemoretz Did you find any solution or workaround?

@naqvitalha naqvitalha added the P1 Important but not urgent label Sep 29, 2022
@taboulot
Copy link

taboulot commented Oct 4, 2022

I'm precisely facing this problem.

Can I help with it in any way ?
@Stevemoretz do you have any news with your investigation?

@Stevemoretz
Copy link
Author

@id3vz @taboulot

Unfortunately I did not find any workarounds or anything, FlatList from react native still has RTL issues, and FlashList still has the same RTL issues nothing has changed, if you really want to help you could take a look at the information I provided and try to do a pull request on this issue.

I couldn't make it work myself, but everything I found out I posted above in this issue.

@MarouaniALA
Copy link

any news on this plz

@Kaveh-ap
Copy link

As in lots of countries, RTL is crucial to develop a proper app, so supporting RTL would be superb.
Unfortunately, we've been struggling with FlatList RTL issues too.

@DevineDecrypter
Copy link

are there any updates on this issue? it`s really a shame to not be able to use this library because of problems with RTL :(

@mbilenko-florio
Copy link

mbilenko-florio commented Jan 31, 2024

Did little investigation about the problem. Looks like problems lies both in the FlashList autolayout code and the underlying recyclerlistview.

So as first step I applied this patch to recyclerlistview https://github.com/Flipkart/recyclerlistview/pull/629/files
That seemingly did not help. Then tried disabling autolayout - it removed gaps between items, but now I have incorrect list order

  <FlashList
          disableAutoLayout={Platform.OS === "android" && I18nManager.isRTL}

@faizazafar
Copy link

faizazafar commented Sep 6, 2024

has anyone found any solution for this RTL issue in FlashList ?
If anyone can help, I am facing the same issue

@siemah
Copy link

siemah commented Sep 16, 2024

Hey guys, @Stevemoretz @faizazafar @Kaveh-ap @MarouaniALA you can always force the layout to render component from left to right even when the device settings is RTL using I18nManager.forceRTL() and I18nManager.swapLeftAndRightInRTL() then you stylize the component according to the RTL and LTR.

@Ozaoujal
Copy link

Ozaoujal commented Oct 4, 2024

There’s no change, the issue persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 Important but not urgent
Projects
None yet
Development

No branches or pull requests