ionic-team / ionic-v3

The repo for Ionic 3.x. For the latest version of Ionic, please see https://github.com/ionic-team/ionic
Other
128 stars 85 forks source link

Ionic slides custom component instances do not behave the same #990

Open ionitron-bot[bot] opened 5 years ago

ionitron-bot[bot] commented 5 years ago

Original issue by @JohnGrisham on 2019-03-13T18:09:05Z

Bug Report

Ionic version: 4.11.0 Current behavior:

When wrapping and ionic-slides component in another custom component different instances of that component do not behave the same. When screen is resized only one of these instances will resize and maintain number of slides per view property. Other slides also do not maintain centering on the screen. When adding only one instance of slides custom component the slides behaves as expected.

Expected behavior: All the instances of the slides component should respond the same the screen resize events.

Steps to reproduce:

Create a component with ionic slides element, export that component and import into a page. Add multiple copies of that component and try to resize the screen to observe responsiveness. When only one slide component exists it will respond well. If there is more than one only one will behave as desired.

Related code:

movieslides.zip home.zip

Home.html

<movie-slider id="nowPlaying" *ngIf="mdata.inTheatres.length > 0" [movies]="mdata.inTheatres"></movie-slider>

  <movie-slider id="nowPlayingNearYou" *ngIf="mdata.nearbyShowings.length > 0" [movies]="mdata.nearbyShowings"></movie-slider>

  <movie-slider id="genreSuggestions" *ngIf="mdata.suggestedByGenre.length > 0" [movies]="mdata.suggestedByGenre" [showGenres]="true"></movie-slider>

Movieslider.html

<ion-slides #Slides class="slides" *ngIf="movies.length > 0" loop="true" centeredSlides="true" slidesPerView="3">
    <ion-slide class="movie card" *ngFor="let movie of movies">
      <div #innerMovie class="inner-movie">
        <a (click)="pushDetails(movie)">
        <div *ngIf="movie.showtimes">
          <div *ngIf="movie.showtimes.length > 0" class="showings">
            <div>Now Playing At: {{movie.showtimes[0].theatre.name}}</div>
            <div class="showtimes">
              <h3 *ngFor="let showtime of movie.showtimes">{{showtime.dateTime | date : 'shortTime'}}</h3>
            </div>
          </div>
        </div>
        <img src="{{movie.poster_path ? 'https://image.tmdb.org/t/p/original' + movie.poster_path : '/assets/imgs/no_image.jpg'}}" class="poster">
        <h3 class="title">{{movie.title}}</h3>
        </a>
        <div #genres *ngIf="genreDisplay" class="genres" [ngClass]="{'scrolling' : checkOverflow(genres, innerMovie)}">
          <h5 *ngFor="let genre of movie.genres; let last = last">{{genre.id ? setGenreName(genre) : genre}}<span *ngIf="!last">&#47;</span></h5>
        </div>
      </div>
        <p class="overview">{{movie.overview ? movie.overview : movie.longDescription}}</p>
    </ion-slide>
  </ion-slides>

Ionic info:

insert the output from ionic info here
![Capture](https://user-images.githubusercontent.com/32438099/54303518-27b10d80-4591-11e9-9607-04a6b5e24779.PNG)
JohnGrisham commented 5 years ago

Capture

longgt commented 5 years ago

Resize handler logic is share same setTimeout id so if there are multiple slides on a page, maybe only one of them was resized properly.

https://github.com/ionic-team/ionic-v3/blob/87d75d7d570ce6f798a7084aa412ec864b391617/src/components/slides/swiper/swiper-events.ts#L837

JohnGrisham commented 5 years ago

@longgt Is there a fix for this that you would suggest?

longgt commented 5 years ago

@JohnGrisham Slides actually has an internal id so you can use it to save/cancel resizeId per slides.

https://github.com/ionic-team/ionic-v3/blob/87d75d7d570ce6f798a7084aa412ec864b391617/src/components/slides/slides.ts#L916

JohnGrisham commented 5 years ago

@JohnGrisham Slides actually has an internal id so you can use it to save/cancel resizeId per slides.

ionic-v3/src/components/slides/slides.ts

Line 916 in 87d75d7

this.id = ++slidesId;

@longgt Upon further testing I noticed that adjusting settings to slides component only applies to the slides element that was created last. Here is an example of my code. I am getting all view children that have the slides reference and getting each of their slides properties. I am expecting that the setSlidesPerView method will apply slidesPerView property changes to all instances of slides but it only applies to the slide with id -2 which is the last created slide.

MovieSliderComponent.html

<ion-slides #Slides class="slides" *ngIf="movies.length > 0" centeredSlides="true" loop="true" slidesPerView="3">

I add a reference to slides in my html.

MovieSliderComponent.ts

@ViewChild('Slides') slider: Slides;

I apply the reference to a variable.

Home.html

<movie-slider #Slider id="nowPlaying" *ngIf="mdata.inTheatres.length > 0" [movies]="mdata.inTheatres">

<movie-slider #Slider id="nowPlayingNearYou" *ngIf="mdata.nearbyShowings.length > 0" [movies]="mdata.nearbyShowings">

<movie-slider #Slider id="genreSuggestions" *ngIf="mdata.suggestedByGenre.length > 0" [movies]="mdata.suggestedByGenre" [showGenres]="true">

I include my component in my page html three times.

Home.ts

@ViewChildren('Slider') Slides: MovieSliderComponent[];

I reference each of my slides in a variable;

setSlidesPerView() {

 if(this.platform.width() >= 1200) {
   this.Slides.forEach((slider) => slider.slider.slidesPerView = 5);
   console.log(this.platform.width());
 }

 else if(this.platform.width() >= 319 && this.platform.width() < 1200) {
   this.Slides.forEach((slider) => slider.slider.slidesPerView = 3);
 }

 else if(this.platform.width() < 319) {
   this.Slides.forEach((slider) => slider.slider.slidesPerView = 1);
 }

 this.cd.detectChanges();

}

I attempt to change each of my sliders slidesPerView option but only the slide with slide-id-2 which is the slider last created gets updated.

I might attempt to remove slider Id or keep them all the same but I imagine that it won't change the result for some reason when dealing with multiple ion-slider elements only the last slider created can be updated via these properties.

longgt commented 5 years ago

@JohnGrisham Could you do some patch on node_modules\ionic-angular\components\slides\swiper\swiper-event.js and check again.

Before

/*=========================
  Resize Handler
  ===========================*/
var resizeId;
function onResize(s, plt, forceUpdatePagination) {
    // TODO: hacky, we should use Resize Observer in the future
    if (resizeId) {
        plt.cancelTimeout(resizeId);
        resizeId = null;
    }
    resizeId = plt.timeout(function () { return doResize(s, plt, forceUpdatePagination); }, 200);
}
function doResize(s, plt, forceUpdatePagination) {
    resizeId = null;

After

/*=========================
  Resize Handler
  ===========================*/
var resizeIdMap = new Map();
function onResize(s, plt, forceUpdatePagination) {
    // TODO: hacky, we should use Resize Observer in the future
    var resizeId = resizeIdMap.get(s.id);
    if (resizeId) {
        plt.cancelTimeout(resizeId);
        resizeIdMap.delete(s.id);
    }
    resizeIdMap.set(s.id, plt.timeout(function () { return doResize(s, plt, forceUpdatePagination); }, 200));
}
function doResize(s, plt, forceUpdatePagination) {
    resizeIdMap.delete(s.id);
JohnGrisham commented 5 years ago

@longgt I appreciate your help with this however I don't believe the resizeId is the issue here. I think resize ID is only used to track the actual event of resizing the screen. I solved this issue by calling resize and then update on each sliders slide component instance.

@Ionitron request close.

longgt commented 5 years ago

@JohnGrisham resizeId is only used to track ... I took a debug on Slides when resize event happened and got that issue. Maybe there is something out of here but it worked for my case. But if you see deep into source code, you see that there is a cancelTimeout logic here and this will stop doResize logic in previous slides instance. So if we have more than 1 instance of slides on same component, only the last component will be updated. I tested with my custom calendar component (based on Ionic Slides) and realized that when doing resize screen width, only the last calendar was resized properly. If I changed logic of resizeId as the above workaround, all of calendar instance was updated properly.