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

Options for Slides #10002

Closed
queejie opened this issue Jan 12, 2017 · 17 comments · Fixed by #10008
Closed

Options for Slides #10002

queejie opened this issue Jan 12, 2017 · 17 comments · Fixed by #10008
Assignees

Comments

@queejie
Copy link

queejie commented Jan 12, 2017

Ionic version: (check one with "x")
[ ] 1.x
[x ] 2.x

I'm submitting a ... (check one with "x")
[x ] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:

Options will not take effect, if I read the changelog correctly. Therefore, no slider options, such as the ones below will be usable, basically ruining my app:

       provider.sliderOptions = {
            direction: 'horizontal',
            pager: true,
            paginationClickable: true,
            preventClicks: false,
            preventClicksPropagation: false,
            freeMode: false,
            nested: true,
            autoHeight: true,
            scrollSupressionThreshold: (screen.availWidth) / 30,
            horizontalDistanceThreshold: (screen.availWidth) / 30,
            paginationBulletRender: function (index, className) {
                // Save paginator so we can move it to the requested placeholder paginator for this set of slides.
                //
                provider.paginator = window.document.querySelector(this.pagination);
                return '<span class="' + className + '">' + (index + 1) + '</span>';
            },
            onInit: (slides: any) => {
                // Move the paginator to the requested placeholder for this set of slides.  This lets us style it and place it in a more consistent position.
                //
                provider.slider = slides;
                if (provider.paginator) {
                    let placeholderName = placeHolderPrefix + provider.indexVariable;
                    let placeholderEl = window.document.getElementById(placeholderName);
                    if (placeholderEl) {
                        placeholderEl.appendChild(provider.paginator);
                    }
                }
            }
        }

Expected behavior:

It is expected that options are supported, and can be passed through to Swiper.

Steps to reproduce:

I have not "upgraded" yet because of this issue, since it is a downgrade, if I understand the changelog.

Other information:

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

Cordova CLI: 6.4.0 
Ionic CLI Version: 2.1.18
Ionic App Lib Version: 2.1.9
ios-deploy version: 1.9.0 
ios-sim version: 5.0.9 
OS: macOS Sierra
Node Version: v7.2.1
Xcode version: Xcode 8.2.1 Build version 8C1002
@mhartington
Copy link
Contributor

While some options are not available, like paginationBulletRender, most of the other options are still accessible.

http://ionicframework.com/docs/v2/nightly/api/components/slides/Slides/#advanced

For example, your configuration might look like

class MyPage {
  @ViewChild(Slides) slides: Slides;

  ngAfterViewInit() {
    this.slides.paginationClickable =  true;
    this.slides.preventClicks = false;
    this.slides.preventClicksPropagation = false;
    this.slides.autoHeight = true;
    this.slides.threshold = (screen.availWidth) / 30
  
  }
}

scrollSupressionThreshold and horizontalDistanceThreshold are actually not valid options for the swiper API

As for paginationBulletRender, what were you using it for?

@mhartington mhartington self-assigned this Jan 12, 2017
@mhartington mhartington added the v2 label Jan 12, 2017
@queejie
Copy link
Author

queejie commented Jan 12, 2017

There are three things that we do. The first is to relocate the bullets to a div below the slides, and place them in a consistent position. Simply styling them with CSS was not enough, because they would be hidden if they stayed within the slide, or not position consistently at the very bottom.
Secondly, we can append next and previous arrows to the left and right of the bullets, because it doesn't seem as though that is supported in the version of Swiper being used.

For those two things we need the onInit(), in terms of timing.

Thirdly, we wanted to render numbers within the bullets.

I just spent two days figuring this stuff out earlier this week so the design group could work with the arrows and bullets. Then suddenly it seems that we can no longer do this.

Thank you.

@mhartington
Copy link
Contributor

I'll take a look about adding this back in.

@queejie
Copy link
Author

queejie commented Jan 12, 2017

slide

@queejie
Copy link
Author

queejie commented Jan 12, 2017

Thanks so much!

@mhartington
Copy link
Contributor

So I have the paginationRender working locally, though I'm confused as to why you could not use the ngAfterViewInit method for the rest of things? It should keep things in sync in terms of timing

@sidjoshi001
Copy link

@mhartington Thanks For this Update & how we can set centeredSlides: true,

below method is Correct ??

class MyPage {
  @ViewChild(Slides) slides: Slides;

  ngAfterViewInit() {
    this.slides.centeredSlides = true;
  }
}

@queejie
Copy link
Author

queejie commented Jan 13, 2017

@mhartington Thank you for adding the access to the renderer.

It's odd that you would be confused by my not using ngAfterViewInit() method. I didn't use it because I wasn't aware of it until you mentioned it here. There is no mention of it in either the documentation or the changelog. How would anyone guess such a thing were possible?

What about the onInit(), which is what we need to relocate the bar so we can add the next/previous arrows. Is that also supported by the ngAfterViewInit() setting? Is there a list of which options are permitted somewhere? I think I read somewhere that accessing the slides as recommended might be deprecated. Is that true? I'd hate to have another surprise like this one.

Thank you.

@queejie
Copy link
Author

queejie commented Jan 13, 2017

@mhartington You mentioned that you "have it working locally", but I'm not sure what that means. Will I have to wait for a next release or figure out how to get a nightly download? It can take several hours to upgrade between ionic releases, so I'm anxious to minimize the number of times that is needed.

@mhartington
Copy link
Contributor

@sidjoshi001 correct. Again, all the possible options that can be set in the slides source.

@WhatsThatItsPat
Copy link

@queejie As Ionic is built on top of Angular, it retains that API as well including Angular's Lifecycle hooks.

@mhartington
Copy link
Contributor

There is no mention of it in either the documentation or the changelog. How would anyone guess such a thing were possible?

It's a standard angular 2 lifecycle hook. Mentioned quite a bit in angular's documentation.

As for onInit, again, ngAfterViewInit should be enough.

Since it's just a function you could do this.

@ViewChild(slides) slider;
 ngAfterViewInit() {
                // Move the paginator to the requested placeholder for this set of slides.  This lets us style it and place it in a more consistent position.
                //
                provider.slider = this.slider;
                if (provider.paginator) {
                    let placeholderName = placeHolderPrefix + provider.indexVariable;
                    let placeholderEl = window.document.getElementById(placeholderName);
                    if (placeholderEl) {
                        placeholderEl.appendChild(provider.paginator);
                    }
                }
            }

Something like that, might need a bit of tweaking.

As for time-frame, if the PR is accepted, it will be in the next release, but we could try to cut a nightly for testing.

@queejie
Copy link
Author

queejie commented Jan 13, 2017

@mhartington I know that. What I meant is that there is no mention of the possibility of setting slide options in that event hook.

@queejie
Copy link
Author

queejie commented Jan 13, 2017

@PatrickMcD It's strange to me that you guys don't see the obvious missing information for developers here, and that it has nothing to do with Angular. I have built substantially large and complex apps in Ionic 1 and 2. When I come across documentation for a parameter called "mode" and it is described as "pass the mode to be used" it is the same sort of thing. It must seem obvious to you what that might mean and what the possible values might be, but not to readers. Likewise, while the Angular event lifecycle is well documented, it is not obvious that options passed to a Swiper creation script could be set, or at which point in the lifecyle that they are set would be meaningful.

@mhartington
Copy link
Contributor

Yes and no actually.
No there is not mention of it in the RC 5 docs (I'll take the blame for that)
Yes there is mention of it in the nightly docs

http://ionicframework.com/docs/v2/nightly/api/components/slides/Slides/#advanced

Totally get the confusion on that part, my fault! 😄

@queejie
Copy link
Author

queejie commented Jan 13, 2017

Okay, thanks. Now I know of another place to look for documentation at least.

@mhartington
Copy link
Contributor

Since the PR has been merged, this will be in the next release.
I'll post here when we cut a new nightly with the renderer in it.

For now, I'm going to lock this issue.

@ionic-team ionic-team locked and limited conversation to collaborators Jan 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants