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

[WIP] add clickableBullets option #27

Merged
merged 1 commit into from Jun 3, 2019

Conversation

raerpo
Copy link

@raerpo raerpo commented Jun 2, 2019

Hi @nickpisacane,

Fixes #25

I did found the option that needs to be added to enable the clickable bullets but I'm not sure of the approach that you prefer for the API.

It seems that the only place to configure the behaviour is through the swiperOptions prop so that's why I put the option clickableBullets but the approach that iDangerous take was to set pagination as a configuration object.

So I want you to tell me what is the right approach for you with this inclusion :)

Thanks for your time!

@nickpisacane
Copy link
Owner

Awesome! Is clickableBullets a supported IDangerous swiper option? If yes, then this looks good. Otherwise, maybe we just make a React property for it? Like paginationClickable or something and add it to the Swiper component. We could just change the line for pagination to something like

Object.assign(opts.pagination, {
        el: this._pagination,
        clickable: this.props.paginationClickable || false
})

My only reasoning for this would be that at some point it would be nice to support TypeScript out of the box, and the swiperOptions prop is directly passed to iDangerous swiper. But, if clickableBullets is already a iDangerous supported option, then I'll merge in as is. Thanks for your work!

@raerpo
Copy link
Author

raerpo commented Jun 2, 2019

Thanks for the fast response :)

Actually the way to enable the clickable pagination is by adding the property clickable: true inside the pagination object (in react-dynamic-swiper pagination is a boolean not an object).

I just came out with the name clickableBullets (I'm not very good at naming :( )

I will change the property name to paginationClickable and extracted from swiperOptions.

@nickpisacane
Copy link
Owner

Awesome! I think the name is great :) I just think that paginationClickable will go perfectly with the existing pagination prop. Great work and thanks for your contributions!

@nickpisacane nickpisacane merged commit b0079d4 into nickpisacane:master Jun 3, 2019
nickpisacane added a commit that referenced this pull request Jun 3, 2019
nickpisacane added a commit that referenced this pull request Jun 3, 2019
@nickpisacane
Copy link
Owner

@raerpo changes are published in v2.0.1 - thanks!

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.

clickable option in pagination is not working
2 participants