-
Notifications
You must be signed in to change notification settings - Fork 298
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 Support proposal (Issue #156) #199
base: master
Are you sure you want to change the base?
RTL Support proposal (Issue #156) #199
Conversation
add a slide index data attribute to each slide upon initialisation. line 103, [].forEach.call(_.slides, function (_) { _.classList.add('glider-slide') }) changed to [].forEach.call(_.slides, function (_,i) { _.classList.add('glider-slide'); _.setAttribute('data-gslide',i) })
slide index attribute
…into NickPiscitelli-master
Great idea, I've been wanting to try and get RTL support in. Have you attempted this solution at all? I'd like to try and give it a shot myself soon. |
Greetings Nick and a Happy new year to you and your loved ones!
yes, I am using the glider library for my WordPress form plugin and one user complained that slider forms were not working for RTL so I managed to get it to work with the above change in my fork. However, I have not tested it for other applications of Glider such as carousels. |
Thank you! A happy new year to you and yours as well! Glad to hear this solution is working for your use case. Would it be possible to provide a link to the slider form, or the plugin itself? I would love to check it out. |
the plugin is the Smart Grid-layout extension for CF7 which is available on the WordPress repo. The Glider js plugin is loaded and used on front-end forms that use the mutli-step silder form design. I have made a small video screencast showing you the form. Download the 3Mb video file to view it in proper resolution, the google drive visualisation is rather low quality for some reason. [EDIT] updated the screencast link, it was pointing to the wrong file. |
df4a48d
to
1358051
Compare
So when this pull request is going to be merged!!! |
Hey @DeeepDev , I think @NickPiscitelli hasn't merged it because my mod has only been tested on normal sliders and not carousels. As such I have feeling it likely doesn't work for all formats of the Glider plugin objects which would explain why it hasn't been merged. |
Hey, sorry, I don't have as much free time as when I first wrote this plugin so I don't get around to checking on things too often. I don't recall the reason for not originally merging this PR but I would want to perform additional testing and documentation would need to be updated as well. @DeeepDev Have you used this patch at all, has the behavior worked as you expected? |
Thanks for this! We're in the process of localizing our site and have utilized your proposed changes to enable RTL in the carousel on our lead generation pages. https://sa.creativesafetysupply.com/content/landing/5S-poster/index.html
|
Proposal for enabling RTL functionality, issue #156
The plugin uses a flexbox to display the slide track. In RTL direction, the flex-direction automatically reverses.
It's therefore a simple matter (I reckon :) of reversing the element scrollLeft directive to ensure the slides display/scroll from right to left.
I have introduced a new option setting,
which by default is 1 and is set to -1 for a rtl element.
Each
_.ele.scrollLeft
directive is then multiplied with the new setting, i.e.this ensures that calculations of slider status remained unchanged, while the scrollTo function has its scroll value also multiplied by the new setting to ensure the scrollLeft directive goes in the correct direction,