-
Notifications
You must be signed in to change notification settings - Fork 711
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
FIX: #767 . #788
FIX: #767 . #788
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for helping fix #767, once you address the comments I can test and verify on my end. 🙏
Noted 👍 |
Applied the changes 👍 |
src/components/ImageGallery.jsx
Outdated
const { infinite } = this.props; | ||
return infinite || this.canSlidePrevious(); | ||
const { infinite, isRTL } = this.props; | ||
//so basically is the list is right to left (isRTL), the logic is inversed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grammar fix here:
// When isRTL is true, the logic is reversed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I would rather have the comment removed because the code you've provided is self-explanatory. But since you left a comment, it will alert a contributor to read it, so it should be clear and concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright. A side note, it's my first time contributing to kindly excuse any inconvenience caused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No inconvenience at all, this is part of the code review process. Thank you for contributing!
fixed issue #767. Fixed to bug of left/right navigators only allowing to scroll to first and last image in RTL mode. Now both nav buttons allow slide in the correct direction.