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.

Sign up for GitHub

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

Jump to bottom

bug: Ineffective attributes on Slides Component #5189

Closed
sjclemmy opened this issue Jan 25, 2016 · 7 comments
Closed

bug: Ineffective attributes on Slides Component #5189

sjclemmy opened this issue Jan 25, 2016 · 7 comments
Assignees
@brandyscarney
Milestone
2.0.0-beta.1

Comments

@sjclemmy
Copy link

Type: bug

Ionic Version: 2.x

Platform: desktop browser

Adding loop="false" doesn't stop the slides looping. Adding index="2" doesn't start the slide from the 2nd (or 3rd if zero indexed?), it always begins with the first.
@Ionitron Ionitron added the v2 label Jan 25, 2016
brandyscarney added a commit that referenced this issue Jan 25, 2016
@brandyscarney
fix(slides): convert loop attribute to a boolean and index to a numbe…
de9a986 
…r before passing to slides

Not sure I like the index attribute though.

Fixes #5189
@brandyscarney
Copy link
Member

These will work in the next release. I'm not sure I like the index attribute though. Thinking we should stick to the same attribute names as the Swiper API.

Thoughts? @adamdbradley @mhartington @drewrygh @tlancina

@brandyscarney brandyscarney self-assigned this Jan 25, 2016
@tlancina
Copy link
Contributor

+1 for using the same attributes as Swiper

@mlynch
Copy link
Contributor

mlynch commented Feb 1, 2016

So, one idea I had from v1 is to not offer attributes at all, but instead have an options field that exposes the entire options of Swiper. Then we don't have to support the mapping for such a complicated component

@brandyscarney
Copy link
Member

Ok so we are going to move forward with removing attributes and supporting passing an options field.

@brandyscarney brandyscarney added this to the 2.0.0-beta.1 milestone Feb 11, 2016
brandyscarney added a commit that referenced this issue Feb 18, 2016
@brandyscarney
refactor(slides): remove the attributes from the slider and use optio…
d21ae88 
…ns to pass them instead

also commented out some attributes that aren’t working right now.

BREAKING CHANGES: Slider attributes have been removed in favor of
passing them in options

references #5189
brandyscarney added a commit that referenced this issue Feb 18, 2016
@brandyscarney
test(slides): update slide tests to use options instead of attributes
ae392e2 
references #5189
@brandyscarney brandyscarney mentioned this issue Feb 18, 2016
Closed
@brandyscarney
Copy link
Member

I'm going to close this issue since all the attributes will now work (in beta.1) by passing them through the options attribute. I'll open a new issue for further slide fixing. Thanks!

@dylanvdmerwe
Copy link
Contributor

Please can the docs be updated for this as they still show the use of attributes which can be confusing.

@brandyscarney
Copy link
Member

@dylanvdmerwe done: ionic-team/ionic-site@5b8a473

Thanks for pointing that out! I missed those earlier. Should be live on the site soon.

@mhartington mhartington mentioned this issue Mar 11, 2016
Closed
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@brandyscarney brandyscarney

Labels
None yet
Projects
None yet
Milestone
2.0.0-beta.1
Development

No branches or pull requests
7 participants
@mlynch @adamdbradley @tlancina @sjclemmy @dylanvdmerwe @brandyscarney @Ionitron