Can't create custom Slide components (ion-slide requires ion-slides in the template)

[jmilloy]

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

I'm submitting a ... (check one with "x") [ ] bug report [x] 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: Wrapping the Slide component in a custom component causes a template parse error because there is no provider for Slides. This is understandable, the Slide component uses a reference to its parent Slides in order to update the Swiper automatically when a new slide is dynamically added or destroyed.

Expected behavior: It would be great to be able to make custom slide components by wrapping Slide. I would be willing to sacrifice the automatic updates when a slide is dynamically added or destroyed.

Related code:

The custom component would be something like this (which gives the template parse error):

@Component({
  selector: 'my-slide',
  template: '<ion-slide> <h1> My Header </h1> <ng-content></ng-content> </ion-slide>',
})
export class MySlide { }

It would be used in a template like this:

<ion-slides>
  <my-slide> Slide 1 </my-slide>
  <my-slide> Slide 1 </my-slide>
</ion-slides>

Other information:

Proposal:

• In the Slide definition, we can remove the Slides constructor argument and the calls to this.slides.rapidUpdate().
• We would then need to call rapidUpdate in Slides.ngAfterViewInit.
• The ion-slide in the custom slide template needs to have the 'slide-zoom' class. It's probably unreasonable to expect users to figure that out, but possibly we can add it manually to all ion-slide elements in Slides.ngAfterViewInit. I haven't tested this.

I think this would allow custom components that wrap ion-slide, but it sacrifices the automatic update when slides are added or removed. That would be okay with me (you can call rapidUpdate yourself, which is what Swiper itself requires). However, perhaps an alternative would be to add an addSlide method and a removeSlide method to Slides, which would call rapidUpdate as necessary. Naively, something like:

function removeSlide(index:number) {
  this.slider.slides[index].remove();
  this.rapidUpdate();
}

function addSlide(elem:HTMLElement, index?:number) {
  if (index === undefined) {
    this.slider.slides.push(elem);
  } else {
    this.slider.slides.splice(index, 0, elem);
  }
  this.rapidUpdate();
}

Ionic info:

--------------------------------
--------------------------------

Your system information:

Cordova CLI: 6.2.0
Gulp version:  CLI version 1.2.1
Gulp local:  
Ionic CLI Version: 2.1.4
Ionic App Lib Version: 2.1.2
OS: Distributor ID: Ubuntu Description: Ubuntu 16.04.1 LTS 
Node Version: v6.2.2

--------------------------------

[jmilloy]

I believe the proposal here would also be a fix for #9778

[jmilloy]

I finally got around to confirming that removing the rapidUpdate calls breaks Angular directives like ngIf. In my first attempts to address this issue I failed to find a more flexible way to give each slide a reference to the parent slides. That may need to be revisited by someone with actual familiarity with Angular.

[jmilloy]

I successfully used DOMNodeInserted and DOMNodeRemoved to update the swiper when slides are added or removed. Are those events reliable?

  // bound to DOMNodeInserted and DOMNodeRemoved
  onDOMChanged(e) {
    if (e.target.classList.contains('swiper-slide')) {
      this.slides.rapidUpdate();
    }
  }

[mhartington]

Hi, is this still an issue with the slides updates in RC5?

As far as using custom slide components, I really don't recommend this as ng2's component model and content project will prevent the expected behavior.

[jmilloy]

Yes, it's still the same because the slide component still requires a reference to the @Host slides component. I'm still getting by with my own slide components and DOMNodeInserted and DOMNodeRemoved callbacks. It's a bit brittle.

[mhartington]

To be honest, I dont think you should be doing this anyways. This feels like over-engineering slides,

Doing

<my-slide>slide 1</my-slide>

to just render

<my-slide>
  <ion-slide>
   Slide 1
 </ion-slide>
</my-slide>

Seems like an extra step that you probably dont need. Going to close as I dont think we should support this.

[jmilloy]

I won't bug again if you still want to close this, but I think you are missing the point.

You have it backwards, but I'll grant that being able to use <my-slide> ... </myslide> instead of <ion-slide> <my-slide> ... </my-slide> </ion-slide> is a small thing.

But because we can't use <ion-slide> in a template without <ion-slides> in the same template, we also can't make reusable components that use <ion-slide>, either.

@Component({
  selector: 'myslides',
  template: '<ion-slides><ng-content></ng-content></ion-slides>',
})
export class MySlides { }

To be used like

<myslides>
  <ion-slide> slide 1 </ion-slide>
  <ion-slide> slide 2 </ion-slide>
</myslides>

I have more than a dozen pages that use slides in the same way, with the same pager, paginationType, and callbacks. Because I've decoupled the Slide component from the Slides component, I can do the above just fine.

I guess I could write a provider with my callbacks, and bind them through a wrapper on every page that uses slides this way, and update all of them anytime anything changes. Or we could find another way to update the slides component when a slide is created or destroyed (so that we can remove @Host slides:Slides and calls to slides.update).

The DOMNodeInserted and DOMNodeRemoved events are working fine for me. A custom event (ionSlideCreated and ionSlideDetroyed, or just ionSlidesUpdated) that is handled by the Slides seems like it would work, too, and better. Probably there are other and even better ways that I haven't thought of.

This change has the side-effect of also allowing us to wrap ion-slide in components, too, which would be nice. I don't see any drawbacks.

[jmilloy]

Just tested the ionSlideUpdated event. I'm happy to put together a pull request if you want to see exactly what I'm proposing.

[mhartington]

What you are trying to do would have been possible in Angular 1, but in 2x it is not since the components are very coupled.

In ng1 we could just do replace and things would be fine. But v2 does not have this. If all you're doing is passing settings to the slides, why not use a directive instead? It has no ties to DOM, and can just decorate the html that you already have (like setting default params for slides).

[jmilloy]

What do you mean that it is not possible in Angular 2? I'm doing it just fine with tweaked versions of the slides components. If you can be more specific about what won't work, I can take a look at it.

[jmilloy]

As for the directive, I'm looking into this approach.

@Directive({ selector: '[myDirective]' })
export class myDirective {
  @Input('myDirective') slides;
  constructor(private el: ElementRef) {}
}

and

<ion-slides #slides [myDirective]="slides">
    ...
</ion-slides>

How do I set the pager attribute using the directive, for example? I tried setting this.slides.pager = true in the directive ngOnInit, but it doesn't actually effect the swiper behaviour. The examples I can find are all just styling the element or otherwise maniuplating the DOM.

Similarly, how do I bind ionSlideWillChange to a method in the directive?

This is a reasonable appreach, and I think this would get me most of the way there. One issue remains, but I'll keep looking into it.

[mhartington]

Seems to work fine here

http://plnkr.co/edit/eO86cb?p=preview

To bind to the events, you just setup you subscriptions in the ngOnInit

[jmilloy]

Maybe you posted the wrong plunker link? I don't see any ion-slides or attribute directives.

[mhartington]

Noticed that after I posted. Updated link

http://plnkr.co/edit/eO86cb?p=preview

[jmilloy]

Alright, great thank you. I've got both those parts sorted out.

There is one more issue, but I'm probably just going to eat this one, it's not too bad. I am loading the initialSlide from storage during ngOnInit, but if storage.get resolves after the slides are finish initializing, then the initialSlide value is ignored. When that happens, I'll have to use slideTo and I'll see the first slide very briefly, which I don't like.

  ngOnInit() {
    this.storage.get(this.id + 'position').then(index => this.slides.initialSlide = index);
  }
}

There's no way to make the slides wait until that promise resolves is there? I am doing it currently with <ion-slides *ngIf="showSlides" ...> and I set showSlides=true only after the promise resolves, but that's not an option anymore with the directive.

Either way, thanks for your help.

[ionitron-bot[bot]]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.