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.94k stars 13.52k forks source link

bug: $ionicSlideBoxDelegate.$getByHandle doesn't work with dynamic value in delegate-handle #5631

Closed dwilt closed 8 years ago

dwilt commented 8 years ago

Short description of the problem:

I'm trying to call slideTo in my controller and it's not working. I've narrowed it down to when the delegate-handle in the template is being dynamically created (as shown in my controller code below), it fires this error when I call slideTo:

Delegate for handle "tour-3b6a5845-29b4-44d3-ad92-06bb9ccecca3-slider" could not find a corresponding element with delegate-handle="tour-3b6a5845-29b4-44d3-ad92-06bb9ccecca3-slider"! slide() was not called!
Possible cause: If you are calling slide() immediately, and your element with delegate-handle="tour-3b6a5845-29b4-44d3-ad92-06bb9ccecca3-slider" is a child of your controller, then your element may not be compiled yet. Put a $timeout around your call to slide() and try again.

If I put a breakpoint inside of the slideTo and inspect the this.$ionicSlideBoxDelegate._instances[0].$$delegateHandle, I can see it's not using the interpolated value, but the controllerVariable and curly brackets: {{ tourSlider.delegateHandle }}. While in this breakpoint, I've confirmed that the delegateHandle I'm using for $getByHandle is indeed the same in both the controller and the template.

I've tried putting slider.slide called in timeout wrappers and using the $ionicSlideBoxDelegate.update() before hand but neither works.

What's also interesting, is that while in this breakpoint, if I call this.$ionicSlideBoxDelegate.slide(0) (without using the $getByHandle), it slides.

What behavior are you expecting?

I'm expecting it to slide.

In my template:

ion-slide-box.tour-slider(
    show-pager="false"
    on-slide-changed="tourSlider.slideChanged($index)"
    delegate-handle="{{ tourSlider.delegateHandle }}"
    active-slide="tourSlider.activeSlideIndex"
)
    ion-slide(
        ng-repeat="slide in mySlider.slides track by slide.id"
    )

in my controller constructor:

this.delegateHandle = `${this.tour.getId()}-slider`;

in slideTo on my controller:

slideTo(slideId) {
    var slide = this.slides.find(slide => slide.id === slideId),
        index = this.slides.indexOf(slide),
        slider = this.$ionicSlideBoxDelegate.$getByHandle(this.delegateHandle);

    slider.slide(index);
}

Ionic Version: 1.2.4-nightly-1917

Browser & Operating System: iOS / Chrome (haven't tested in Android)

Run ionic info from terminal/cmd prompt:

Your system information:

Cordova CLI: 6.0.0
Gulp version:  CLI version 3.9.0
Gulp local:   Local version 3.9.0
Ionic CLI Version: 1.7.13
Ionic App Lib Version: 0.6.5
ios-deploy version: 1.8.4 
ios-sim version: 5.0.4 
OS: Mac OS X El Capitan
Node Version: v5.1.1
Xcode version: Xcode 7.2.1 Build version 7C1002 
meticoeus commented 8 years ago

This is a feature request. The current delegate handle implementation does not support dynamic handles. The delegate currently must exist when the slidebox directive is initialized.

You can work around this by editing the ionSlideBox directive in ionic-angular.js Add delegateHandle: '@' to the scope: {} definition

And change the $ionicSlideBoxDelegate._registerInstance parameter from $attrs.delegateHandle to $scope.delegateHandle

This work around only applies if the scope value assigning the delegate exists when the directive is instantiated. If you change it afterwards, expect bugs.

dwilt commented 8 years ago

@meticoeus The issue with that would be of course that we use bower to manage all of our dependencies, so every time someone would do a bower install they would be overwriting my local patch, no? If this is that easy, is it not something that can be merged in with a new Ionic release?

meticoeus commented 8 years ago

@dwilt You can fork Ionic, commit the changes in your fork, and point bower at the fork instead by passing the full git url rather than the repository name. Now everyone on your team that runs bower install will get the fork instead. Just pull changes into your fork when an update is released.

jgw96 commented 8 years ago

Thanks for opening this issue! If you have a patch for this issue i would definitely recommend opening a pull request for it, that would be awesome! At this time im gonna go ahead and close this issue, but feel free to reopen this issue if needed!