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

Fix Slider Next action when per-view is added #58

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

junaidbhura
Copy link
Contributor

Fix Slider Next action when per-view is added.

@benerd benerd marked this pull request as ready for review September 5, 2024 11:47
Copy link
Contributor

@benerd benerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, @junaidbhura please can you please review this?

@junaidbhura
Copy link
Contributor Author

Thanks @benerd have you tested this on ET?

@harshdeep-gill
Copy link
Contributor

The issue this PR addresses occurs when the per-view attribute is set to a value greater than 1. While navigating to the last item by pressing the next button, the current logic only checks if the current slide index is on the last item before stopping the slide.

However, with a per-view value (e.g., 2), the last item is already in view when the current slide index is on the second-to-last item. Therefore, the slider should not continue advancing. Currently, this results in empty spaces appearing at the end of the slider.

Screen.Recording.2024-09-06.at.10.05.34.AM.mov

@sagarjadhav
Copy link
Contributor

@harshdeep-gill Can you please provide the sample usage code with settings?

Copy link
Contributor

@sagarjadhav sagarjadhav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harshdeep-gill Thanks for the call. As per the discussion and findings we need to fix following issues.

Need to add disable attributes to Previous and Next buttons when slides end.
Pagination should work properly with the steps.
cc: @junaidbhura

@harshdeep-gill
Copy link
Contributor

Hi @sagarjadhav , As discussed there were two more issues that aroused with fixing the per-view issue - those were, slider-nav buttons were not in sync with step variable and while moving to the last image next button was not getting the disabled:"yes" attribute. In the latest commit these two issues are being fixed.
cc: @junaidbhura

@sagarjadhav sagarjadhav self-requested a review September 12, 2024 06:09
Copy link
Contributor

@sagarjadhav sagarjadhav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@junaidbhura junaidbhura merged commit dec657c into master Sep 12, 2024
1 check passed
@junaidbhura junaidbhura deleted the fix/slider-per-view branch September 12, 2024 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants