react-slick icon indicating copy to clipboard operation
react-slick copied to clipboard

WIP Rtl fixes

Open mcfoton opened this issue 7 years ago • 4 comments

Hey guys. In relation to issue 1227 I believe this is not a full PR, but that's sure a start. If you can comment on points listed below I think I'll be able to finish it. Or maybe you'll crack it fast and efficient with this tip.

  1. Fixes initial load and enabling\disabling of arrows for sliders with slidesToScroll: 1
  2. I didn't check any other modes (vertical, slideToScroll > 1, center mode ...)
  3. Architecture \ code style : I kept current coding style, but if condition now look ugly. Maybe use smth like let disabled and make several if-else statements that changed it to true and after that a check for value of this variable. I.e. here
  4. Architecture \ code style : maybe check for previous arrow should be moved to a separate method, same as for the nextArrow?
  5. I'm not sure the places I've changed are the right places to change. Maybe the check should be somewhere else, changing currentSlide value for rtl cases or something like that.
  6. How do I proceed from here? :)

mcfoton avatar Jun 25 '18 07:06 mcfoton

just noticed some diffs with brackets styling, sorry for that. can re-submit if needed. also the fix surely doesn't work with variableWidth = true

mcfoton avatar Jun 25 '18 10:06 mcfoton

Can this PR be merged now? Facing the same issue mentioned in 1227

amaankulshreshtha avatar Jun 19 '19 06:06 amaankulshreshtha

@mcfoton How to get rtl working with variableWidth? I mean, the extra spacing on the right side of first item.

sanishkr avatar Nov 25 '19 10:11 sanishkr

@mcfoton How to get rtl working with variableWidth? I mean, the extra spacing on the right side of first item.

@sanishkr Never got it working :(

mcfoton avatar Nov 25 '19 14:11 mcfoton