ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.93k stars 13.52k forks source link

bug: ion-slides loop removes bindings such as onClick and routerLink when looping #22414

Closed kas84 closed 2 years ago

kas84 commented 3 years ago

Bug Report

Ionic version:

[ ] 4.x [x] 5.x

Current behavior:

Expected behavior:

The one that is active has an 'active' class that makes the letter bold. By clicking any button, you should make the clicked button active.

Related code:

https://github.com/kas84/ion-slides-vue-sample

Other information:

Ionic info:

Ionic:

   Ionic CLI       : 6.12.0 (/Users/pablo/.nvm/versions/node/v14.7.0/lib/node_modules/@ionic/cli)
   Ionic Framework : @ionic/vue 5.4.1

Capacitor:

   Capacitor CLI   : 2.4.2
   @capacitor/core : 2.4.2

Utility:

   cordova-res : 0.15.1
   native-run  : not installed

System:

   NodeJS : v14.7.0 (/Users/pablo/.nvm/versions/node/v14.7.0/bin/node)
   npm    : 6.14.8
   OS     : macOS Catalina
kas84 commented 3 years ago

This is most likely a problem with ion-slides on the stencil component, since with ion-slides on react fails as well

https://github.com/kas84/ion-slides-react-sample

kas84 commented 3 years ago

Fails as well on ionic-angular

https://github.com/kas84/ion-slides-bug-angular

kas84 commented 3 years ago

Am I missing something on this issue? Works okay when loop=false, but not when loop=true, but on ionic-angular I've seen ionic2-calendar work just fine, which uses ion-slides with loop option set to true under the hood

claviska commented 3 years ago

I took a look at your test case and, when loop is true, Swiper creates new nodes to facilitate the looping effect. As a result, event listeners and such that get attached to the original slide are lost.

You can work around this by listening higher up, such as on the <ion-slides> element. Keep in mind the event target won't necessarily be the original element, so you'll need to pass the index in something like a data-index attribute.

I'm transferring this issue to the docs so we can add a "gotcha" about this.

liamdebeasi commented 2 years ago

Thanks for the issue!

Starting with Ionic 5.7.0, we deprecated ion-slides in favor of having developers install Swiper.js directly in their projects. This gives developers access to new features, an improved developer experience, and bug fixes that they did not have access to when using ion-slides.

Since ion-slides uses Swiper.js under the hood, this migration should allow the functionality of your slides to remain exactly the same.

As a result of this deprecation, we have suspended any new development on ion-slides with the exception of critical fixes, such as resolving security issues.

I am going to close this issue out, but we strongly encourage you to try out the latest version of Swiper.js in your project. We have found that upgrading to the latest version of Swiper.js resolved a large number of common problems that Ionic developers were running into.

Please see https://ionicframework.com/docs/api/slides for more information on this change as well as a step by step migration guide for your apps. Developers will have approximately 1 year to migrate, starting from the initial deprecation in September 2021.

I understand this may not be the immediate resolution you were hoping for, but we think this change will significantly improve the developer experience in the long run by placing control of slides back in the hands of the developers.

ionitron-bot[bot] commented 2 years ago

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.