chrizuuu / react-native-gallery-preview

Performant component for gallery previews with smooth gestures interactions and animations 🏞️
MIT License
95 stars 6 forks source link

Support RTL #7

Closed ameer-taghavi closed 2 months ago

ameer-taghavi commented 2 months ago

as titled

chrizuuu commented 2 months ago

Hi, I was planning to add RTL support in version 1.2. There will be one or two more smaller releases before that. If time permits I will release everything later this month.

chrizuuu commented 2 months ago

Hi, I just added a new version with RTL support (1.2.0). If you have any comments, please feel free to contact me

ameer-taghavi commented 2 months ago

Hi, thanks man, I'll check it out

ameer-taghavi commented 2 months ago
  1. It only shows the first image
  2. swiping right to show the next image doesn't work
  3. swiping left doesn't work

https://github.com/user-attachments/assets/4f5008a0-619d-4d9a-a4bf-0b6bea1f4cd5

chrizuuu commented 2 months ago

Hi, I tested it again now on both IOS and Android on the current version (1.2.0) and it works fine with me

Are you sure to set the rtl props to true?. I have provided an example below

export const RTLGalleryPreview = () => {
  const [isVisible, setIsVisible] = React.useState(false);

  const handleOpenImageViewer = React.useCallback(() => {
    setIsVisible(true);
  }, []);

  const handleCloseImageViewer = React.useCallback(() => {
    setIsVisible(false);
  }, []);

  return (
    <ExampleWrapper
      title="RTL example”
      buttonLabel="Open RTL example”
      onPress={handleOpenImageViewer}
    >
      <GalleryPreview
        isVisible={isVisible}
        onRequestClose={handleCloseImageViewer}
        images={images}
        rtl
      />
    </ExampleWrapper>.
  );
};

https://github.com/user-attachments/assets/c3f30099-355a-461c-87ce-79af7090b4b5

ameer-taghavi commented 2 months ago

do you set I18nManager.allowRTL(true) and I18nManager.forceRTL(true) in CLI app and

// app.json
 "extra": {
     "supportsRTL": true,
      "forcesRTL": true
}

in expo? my app is RTL layout

ameer-taghavi commented 2 months ago

yo can check I18nManager.isRTL to set rtl in source, no need set rtl props by user

chrizuuu commented 2 months ago

Yhm, okay, I had a different idea for this at first - I wanted to leave it to the developers to control but using i18manager directly might be better -. 1.2.1

ameer-taghavi commented 2 months ago

thanks Krzysztof, but 1.2.1did not fixed bug! please change the app layout to RTL then test it 🙏🏻

chrizuuu commented 2 months ago

I tested this on both regular layout and RTL enabled. I've provided a branch with a finished example on which I tested this https://github.com/chrizuuu/react-native-gallery-preview/tree/fix/rtl

ameer-taghavi commented 2 months ago

yes man, it's ok 🙏🏻 Can you do to reverse swipe?

chrizuuu commented 2 months ago

hmm what exactly do you mean?

I never had any contact with RTL layout also let me use you lightly :)

ameer-taghavi commented 2 months ago

In the RTL layout, we drag the slider from right to left to show the next item. Exactly the opposite of LTR

chrizuuu commented 2 months ago

Good damm it :D It's weird !

I have a request, can you test a sample application from this branch? (https://github.com/chrizuuu/react-native-gallery-preview/tree/fix/rtl). Rtl Example.

ameer-taghavi commented 2 months ago

i fixed in GalleryPreview.tsx file:

please do this changes:

line 71: transform: [{ translateX: translateX.value }],

line 89: return i * -(Dimensions.get("window").width + gap) * 1;

line 195: container: { flexDirection: I18nManager.isRTL "row-reverse" : "row"},

chrizuuu commented 2 months ago

hmm, this is not likely to be correct, because then the layout in RTL looks and works identically to that in LTR

transform: [{ translateX: rtl ? -translateX.value : translateX.value }], Here, this value inversion was added in order for the reverse swipe to work correctly.

line 195: container: { flexDirection: I18nManager.isRTL "row-reverse" : "row"}, This part with RTL enabled makes no sense because it reverses what it automatically does with the view i18manager

return i -(Dimensions.get("window").width + gap) 1; Here I will agree, there was a mistake

chrizuuu commented 2 months ago

I tested and this is how it looks to me after your fixes :)

https://github.com/user-attachments/assets/a7a10bf9-1998-418d-bd55-3e9af702765a

ameer-taghavi commented 2 months ago

we needs this in RTL exactly. but you right, my changes not use in ltr and i will fix it

chrizuuu commented 2 months ago

But what I posted was its own example with RTL layout enabled .

hmm I don't know now, maybe I didn't understand something but is this gallery for RTL supposed to work exactly the same as in LTR? or what is the point:)

EDIT: I just tested the IOS app on Arabic (RTL) And there the only difference that is there is the rendering of gallery items from right to left. Gestures etc stay the same so I leave it as it is currently in version 1.2.1

ameer-taghavi commented 2 months ago

Krzysztof, i waz confused 😂 every things is ok in 1.2.1 and no need to reverse swipe

chrizuuu commented 2 months ago

Ok :D, then I'm closing the issue