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

no RTL support :( #26

Closed
EyalSi opened this issue Feb 18, 2018 · 22 comments
Closed

no RTL support :( #26

EyalSi opened this issue Feb 18, 2018 · 22 comments

Comments

@EyalSi
Copy link

EyalSi commented Feb 18, 2018

Is there a way to activate RTL? seems like it's not working.

@Jacse
Copy link
Owner

Jacse commented Feb 18, 2018

I'm afraid I don't have any experience with RTL layouts. How do you normally do it? I would think setting the titleStyle and textStyle with text-align: 'right' would work.

@EyalSi
Copy link
Author

EyalSi commented Feb 18, 2018

It's more complicated than that ;)
you can check the behavior by setting:
import {
I18nManager,
} from 'react-native';

I18nManager.forceRTL(true);

which messes up the page dots, swipes etc.
I have added a function -

_getRTLPageOffset(pageNum){
return I18nManager.isRTL?(this.props.slides.length-1-pageNum):pageNum;
}
and added the function to this 2 lines:
let newIndex = this._getRTLPageOffset(Math.round(offset / this.state.width));
this.flatList.scrollToOffset({ offset: this._getRTLPageOffset(pageNum) * this.state.width });

It seems to work fine now...

@Jacse
Copy link
Owner

Jacse commented Feb 19, 2018

Thanks for the explanation! I have a few clarifying questions if that's okay (I have no experience with RTL myself):

  • Should the sliders move in the opposite direction? I.e. right -> left instead of left -> right
  • Should the positioning of the next/prev button and the skip/done button be reversed?
  • Should RTL be defined through a prop or should the component check using I18manager do you think?

I hope to add support soon.

@EyalSi
Copy link
Author

EyalSi commented Feb 19, 2018

Thanks for the effort!
In general in RTL, everything should be opposite.
Saying that :) -

Should the sliders move in the opposite direction? I.e. right -> left instead of left -> right

  • Yes
    Should the positioning of the next/prev button and the skip/done button be reversed?
  • Yes
    Should RTL be defined through a prop or should the component check using I18manager do you think?
    The component should check I18nManager. Setting RTL is done through the OS by setting language to Hebrew or Arabic for example. In this case, I18nManager.isRTL will return true.

I hope to add support soon. - THANKS!!

@Jacse
Copy link
Owner

Jacse commented Feb 19, 2018

Got it, thanks! I will hopefully find the time to look into this this week or the next 👍

@EyalSi
Copy link
Author

EyalSi commented Apr 29, 2018

Hi Jacse,
Did you have the time to fix RTL?
10x,

@Jacse
Copy link
Owner

Jacse commented Apr 30, 2018

@EyalSi I'm afraid it proved more difficult than anticipated. Apparently there are some issues with horizontal flatlist on RTL-layouts (see #1996, 13100). Help & PRs welcome.

@EyalSi
Copy link
Author

EyalSi commented May 13, 2018

Hi @Jacse,
I actually have my app productive ;) work with your library and the fixes I wrote above... working in RTL.
Did you face the issues described in 1996 or 13100?

Thanks,
EyalS

@AlirezaAkbarix
Copy link

@EyalSi Thanks for your solution, it works perfectly :)

@AlirezaAkbarix
Copy link

AlirezaAkbarix commented Jul 24, 2018

@EyalSi Oh wait!

It makes the same bug on iOS! so you need to check if Platform.OS === 'android' do the stuff.

@Jacse
Copy link
Owner

Jacse commented Jul 26, 2018

@EyalSi do you think you'd have time to submit a PR with your changes? I'm a bit swarmed atm and it would be highly appreciated.

@EtayM
Copy link

EtayM commented Jul 28, 2018

@EyalSi thank you very much for sharing your fix. worked like a charm for me!

@yogevlahyani
Copy link
Contributor

yogevlahyani commented Nov 6, 2018

Hi, I opened a PR for RTL support :)
I didn't test it on IOS yet, I will in a couple of days, you are all are welcome to test it :)

#72

*** EDIT ***
Seems like it's working properly on IOS.

@Jacse
Copy link
Owner

Jacse commented Nov 9, 2018

Experimental support has landed in 4849fd8. Would love if people could test it out by installing npm i git+https://github.com/Jacse/react-native-app-intro-slider.git

@rgouzal
Copy link

rgouzal commented Jan 20, 2019

Experimental support has landed in 4849fd8. Would love if people could test it out by installing npm i git+https://github.com/Jacse/react-native-app-intro-slider.git

I think this has a bug, I had 3 pages to show on the slider and I had problems when I enabled a language that has RTL true, the problems happens on multiple scenarios when clicking next button, the previous button or when swiping slides.

When clicking on next button:

The slider will go through the next page which is the page with index 1, but when clicking next button at the state of current index 1 it goes all back to the previous page which is page index 0 and the done button show up.

When swiping:

It will go all through the right indexes, but when reaching the last index which is 2 the done button won't show up, but when swiping all back to the first index which is 0 the done button show up.

When clicking on previous:

It will not go to the right index.

This behaviour happens on Android, haven't tested iOS.

@yogevlahyani
Copy link
Contributor

Experimental support has landed in 4849fd8. Would love if people could test it out by installing npm i git+https://github.com/Jacse/react-native-app-intro-slider.git

I think this has a bug, I had 3 pages to show on the slider and I had problems when I enabled a language that has RTL true, the problems happens on multiple scenarios when clicking next button, the previous button or when swiping slides.

When clicking on next button:

The slider will go through the next page which is the page with index 1, but when clicking next button at the state of current index 1 it goes all back to the previous page which is page index 0 and the done button show up.

When swiping:

It will go all through the right indexes, but when reaching the last index which is 2 the done button won't show up, but when swiping all back to the first index which is 0 the done button show up.

When clicking on previous:

It will not go to the right index.

Try this
https://github.com/yogevlahyani/react-native-app-intro-slider

@rgouzal
Copy link

rgouzal commented Jan 20, 2019

Experimental support has landed in 4849fd8. Would love if people could test it out by installing npm i git+https://github.com/Jacse/react-native-app-intro-slider.git

I think this has a bug, I had 3 pages to show on the slider and I had problems when I enabled a language that has RTL true, the problems happens on multiple scenarios when clicking next button, the previous button or when swiping slides.
When clicking on next button:
The slider will go through the next page which is the page with index 1, but when clicking next button at the state of current index 1 it goes all back to the previous page which is page index 0 and the done button show up.
When swiping:
It will go all through the right indexes, but when reaching the last index which is 2 the done button won't show up, but when swiping all back to the first index which is 0 the done button show up.
When clicking on previous:
It will not go to the right index.

Try this
https://github.com/yogevlahyani/react-native-app-intro-slider

Still doing the same behavior

@rgouzal
Copy link

rgouzal commented Jan 20, 2019

@yogevlahyani This is what happens

When clicking the next button and previous button:
rtl-bug

And this is what happens when swiping:

rtl-bug-swipe

@yogevlahyani
Copy link
Contributor

@0x01Brain Have you tried I18nManager.forceRTL(true) ?
Also, try to delete node_modules and install my github version

@rgouzal
Copy link

rgouzal commented Jan 20, 2019

@yogevlahyani Thank you, sir, now it works without any problems. I had to delete node_modules as you said before installing your github version. I thought that doing the following would be enough:

npm uninstall --save react-native-app-intro-slider

@yogevlahyani
Copy link
Contributor

@yogevlahyani Thank you, sir, now it works without any problems. I had to delete node_modules as you said before installing your github version. I thought that doing the following would be enough:

npm uninstall --save react-native-app-intro-slider

Great, happy to help 👍

@Jacse
Copy link
Owner

Jacse commented Mar 3, 2019

This is now landed in master 🍾

@Jacse Jacse closed this as completed Mar 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants