-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
WIP: slidebox #2288
WIP: slidebox #2288
Conversation
ba10540
to
0ba5876
Compare
Nice! One thought as we test that I want to make sure we do right is carrying the velocity with the swipe. slide.js did a good job at this, but if you "throw" quickly, the animation should keep going at that speed. Some slide components "catch" the throw then do an awkward slide after that (see the Ratchet slider for an example of how not to do it: http://goratchet.com/components/#sliders) |
Yeah, it's definitely needed. I'll implement it - it's as simple as transitionDuration = slideWidth / velocity
|
8765f41
to
2bb8e5b
Compare
This is about done. It's very modular, and as a result it's almost ready to be brought into tabs. There's a logic in SlideController that could be moved out into something more reusable. Once that is done, tabs can be refactored for swipeability and these performance improvements. |
7ef84bd
to
dcac7f9
Compare
Was wondering about the todo item "Fix CSS height problem". Will it be possible to setup the slides height so that it is always as tall as the screen (minus the height of header and other content outside of the slide)? Hope this makes sense, and again looking forward to this update :) |
Hi Chris, That problem may be fixed now. I have some new changes to push, I'll test out your use case.
|
Have to add I am eagerly awaiting this as well. I am in the same boat as Chris :) |
The slidebox now alwayss take up the size of its parent scroll container. This is ready for review. |
Closes #2336. Closes #2317. Closes #2290. Closes #2228. Closes #2067. Closes #1890. Closes #1865. Closes #1850. Closes #1755. Closes #1688. Closes #1578. Closes #1501. Closes #1353. Closes #1342. Closes #782. Closes #416. Closes #2288. BREAKING CHANGE: The slideBox's API has undergone many changes. - **`<ion-slide-box>`** attributes have changed (see [documentation](http://ionicframework.com/docs/api/directive/ionSlideBox)): * `active-slide` has changed to `selected`. Change your code from this: ```html <ion-slide-box active-slide="activeSlideIndex"></ion-slide-box> ``` To this: ```html <ion-slide-box selected="activeSlideIndex"></ion-slide-box> ``` * `does-continue` has changed to `loop`. Change your code from this: ```html <ion-slide-box does-continue="shouldLoop"></ion-slide-box> ``` To this: ```html <ion-slide-box loop="shouldLoop"></ion-slide-box> ``` * `auto-play` and `slide-interval` have been merged into `auto-play`. Change your code from this: ```html <!-- autoPlay is on --> <ion-slide-box auto-play="true" slide-interval="1000"> </ion-slide-box> <!-- autoPlay is off --> <ion-slide-box auto-play="false" slide-interval="1000"> </ion-slide-box> ``` To this: ```html <!-- autoPlay is on --> <ion-slide-box auto-play="1000"></ion-slide-box> <!-- autoPlay is off --> <ion-slide-box auto-play="false"></ion-slide-box> ``` * `show-pager` and `pager-click` have been removed. Use a child `<ion-slide-pager>` element. See the [`ion-slide-pager` documentation](http://ionicframework.com/docs/api/directive/ionSlidePager). Change your code from this: ```html <!-- pager using default click action --> <ion-slide-box show-pager="true"> </ion-slide-box> <!-- pager with custom click action --> <ion-slide-box show-pager="true" pager-click="doSomething(index)"> </ion-slide-box> ``` To this: ```html <ion-slide-box> <!-- pager using default click action --> <ion-slide-pager></ion-slide-pager> </ion-slide-box> <ion-slide-box> <!-- pager with custom click action --> <ion-slide-pager ng-click="doSomething(index)"></ion-slide-pager> </ion-slide-box> ``` - **`$ionicSlideBoxDelegate`** methods have changed (see [documentation](http://ionicframework.com/docs/api/service/$ionicSlideBoxDelegate)): - `update()` has been removed. slideBox updates on its own now. - `stop()` has been removed. See `autoPlay()` below. - `start()` hass been removed. See `autoPlay()` below. - `slide(newIndex[, speed])` has been renamed to `select(newIndex[, speed]); - `currentIndex()` has been renamed to `selected()`. - `slidesCount()` has been renamed to `count()`. - New method `$ionicSlideBoxDelegate.autoPlay()`. Change your code from this: ```js // stop auto sliding $ionicSlideBoxDelegate.stop(); // later... start auto sliding $ionicSlideBoxDelegate.start(); ``` To this: ```js var autoPlaySpeed = 3000; //wait 3000 seconds between changing slide // stop auto sliding $ionicSlideBoxDelegate.autoPlay(false); // later... start auto sliding $ionicSlideBoxDelegate.autoPlay(autoPlaySpeed); ``` - `previous()` now returns the index of the previous slide and does not select. Change your code from this: ```js // select previous slide $ionicSlideBoxDelegate.previous(); ``` To this: ```js // select previous slide $ionicSlideBoxDelegate.select( $ionicSlideBoxDelegate.previous() ); ``` - `next()` now returns the index of the next slide and does not select. Change your code from this: ```js // select next slide $ionicSlideBoxDelegate.next(); ``` To this: ```js // select next slide $ionicSlideBoxDelegate.select( $ionicSlideBoxDelegate.next() ); ```
@ajoslin This is great! However, I can't seem to set a start slide. In test/html/slideBox.html adding the following: $scope.$root.selectedIndex = 5; doesn't make a difference. This is because this $watch expression happens after this variable is already set. Executing SlideBoxCtrl.select at that point doesn't seem to work because slides are added afterwards. The other issue I noticed was loading performance. The slide box takes ~500ms to load on a top of the line Macbook Pro. It takes several seconds on a Nexus 5. I'm not exactly sure what's going on internally with Angular, but Request Animation Frame is being fired with each item in the list and then a frame is fired for each item afterwards. See attached timeline: Here Is the timeline export from Chrome DevTools. Thanks for all your hard work! |
@redeemedfadi: the load-time is because the slideBox test has a 1000-item ng-repeat. Also, be sure to run $scope.$apply after changing selectedIndex! (shortcut: The $watch you see in the code is on the isolate scope's selectedIndex variable, which is set when the parent scope's attribute changes. |
..And the requestAnimationFrames look to be from angular-animate running an ng-repeat enter animation for every slide element. |
@ajoslin Thanks for the response. I still can't get the slide box to open on a certain slide. Changing a slide by modifying selectedIndex works fine, but I want to open/initialize on a certain slide. |
@redeemedfadi I see. OK, this can be done. |
@redeemedfadi OK, added. Try out the nightly builds. The slideBox now allows you to set the Then, whenever a new slide is added, it will check if that slide's new index matches what should be the selected index. If so, that slide will be selected. |
Thanks @ajoslin for the quick update! That works great. I also added an ng-if to the slide to greatly reduce the load time with a large number of slides: <ion-slide ng-repeat="i in items" ng-if="$index <= $root.selectedIndex + 2 && $index >= $root.selectedIndex - 2"> This should probably be part of the ionSlide directive, but I'm not familiar enough with angular to know exactly where in the directive it should go. Of course, it doesn't work quite right since the |
@fadi unfortunately it's not that simple, since on-demand compiling of a In the future, we'll have to do something similar to collection-repeat. I On Thu, Oct 9, 2014 at 9:11 AM, Fadi Eliya [email protected] wrote:
|
@ajoslin The I added this to slideBox.html example to test: <ion-slide-box on-slide-changed="onSlideChanged($slideIndex)"> $scope.onSlideChanged = function($index){
console.log($index);
} Thanks! |
@ajoslin I noticed you fixed the on-slide-changed attribute. Thanks! This may be premature, but just wanted to let you know if you didn't notice that all the ion-slides are now being rendered whether or not their state is 'detached'. |
Another bug I found: neither |
Is everybody reading the migration notes? If you open the nightlies docs
|
Ahh sorry I missed that one. I did read the commit notes but forgot about that method. |
Closes #2336. Closes #2317. Closes #2290. Closes #2228. Closes #2067. Closes #1890. Closes #1865. Closes #1850. Closes #1755. Closes #1688. Closes #1578. Closes #1501. Closes #1353. Closes #1342. Closes #782. Closes #416. Closes #2288. BREAKING CHANGE: The slideBox's API has undergone many changes. - **`<ion-slide-box>`** attributes have changed (see [documentation](http://ionicframework.com/docs/api/directive/ionSlideBox)): * `active-slide` has changed to `selected`. Change your code from this: ```html <ion-slide-box active-slide="activeSlideIndex"></ion-slide-box> ``` To this: ```html <ion-slide-box selected="activeSlideIndex"></ion-slide-box> ``` * `does-continue` has changed to `loop`. Change your code from this: ```html <ion-slide-box does-continue="shouldLoop"></ion-slide-box> ``` To this: ```html <ion-slide-box loop="shouldLoop"></ion-slide-box> ``` * `auto-play` and `slide-interval` have been merged into `auto-play`. Change your code from this: ```html <!-- autoPlay is on --> <ion-slide-box auto-play="true" slide-interval="1000"> </ion-slide-box> <!-- autoPlay is off --> <ion-slide-box auto-play="false" slide-interval="1000"> </ion-slide-box> ``` To this: ```html <!-- autoPlay is on --> <ion-slide-box auto-play="1000"></ion-slide-box> <!-- autoPlay is off --> <ion-slide-box auto-play="false"></ion-slide-box> ``` * `show-pager` and `pager-click` have been removed. Use a child `<ion-slide-pager>` element. See the [`ion-slide-pager` documentation](http://ionicframework.com/docs/api/directive/ionSlidePager). Change your code from this: ```html <!-- pager using default click action --> <ion-slide-box show-pager="true"> </ion-slide-box> <!-- pager with custom click action --> <ion-slide-box show-pager="true" pager-click="doSomething(index)"> </ion-slide-box> ``` To this: ```html <ion-slide-box> <!-- pager using default click action --> <ion-slide-pager></ion-slide-pager> </ion-slide-box> <ion-slide-box> <!-- pager with custom click action --> <ion-slide-pager ng-click="doSomething(index)"></ion-slide-pager> </ion-slide-box> ``` - **`$ionicSlideBoxDelegate`** methods have changed (see [documentation](http://ionicframework.com/docs/api/service/$ionicSlideBoxDelegate)): - `update()` has been removed. slideBox updates on its own now. - `stop()` has been removed. See `autoPlay()` below. - `start()` hass been removed. See `autoPlay()` below. - `slide(newIndex[, speed])` has been renamed to `select(newIndex[, speed]); - `currentIndex()` has been renamed to `selected()`. - `slidesCount()` has been renamed to `count()`. - New method `$ionicSlideBoxDelegate.autoPlay()`. Change your code from this: ```js // stop auto sliding $ionicSlideBoxDelegate.stop(); // later... start auto sliding $ionicSlideBoxDelegate.start(); ``` To this: ```js var autoPlaySpeed = 3000; //wait 3000 seconds between changing slide // stop auto sliding $ionicSlideBoxDelegate.autoPlay(false); // later... start auto sliding $ionicSlideBoxDelegate.autoPlay(autoPlaySpeed); ``` - `previous()` now returns the index of the previous slide and does not select. Change your code from this: ```js // select previous slide $ionicSlideBoxDelegate.previous(); ``` To this: ```js // select previous slide $ionicSlideBoxDelegate.select( $ionicSlideBoxDelegate.previous() ); ``` - `next()` now returns the index of the next slide and does not select. Change your code from this: ```js // select next slide $ionicSlideBoxDelegate.next(); ``` To this: ```js // select next slide $ionicSlideBoxDelegate.select( $ionicSlideBoxDelegate.next() ); ```
So this never made it into 1.0 ? |
@ajoslin Any updates to this, and in particular re: lazy loading of slides? |
Yes that's especially what I'm interested in @subeebot I have a large slidebox and sometimes I transition to the view and slide to (for example) slide 100. Unfortunately it loads slides 1-99 as well resulting in a large delay. I'd love to see "Only current, previous, and next element are ever in the DOM" but it seems this pull request was never accepted :( |
Yeah, I'm still dealing with huge load times that make my app appear On Wed, May 27, 2015 at 11:58 PM, bjammin [email protected] wrote:
Marc Syp, Creative Technologist WARNING: The information contained in this message is legally |
I did find this post: It discusses some methods (code included) to implement your own lazy loading. I'll be going down this path I guess. |
Thank you for that! Will give it a try when I have a chance and report Marc On Thu, May 28, 2015 at 5:05 PM, bjammin [email protected] wrote:
Marc Syp, Creative Technologist WARNING: The information contained in this message is legally |
I was trying to use slidebox to re-create this image carousel as pure ionic: Unfortunately I haven't been able to get near the performance of that component, even with a small number of slides. So I have been forced to adapt blueimp instead. It's a bit messy with no direct binding but I have managed to work around it and the performance is streets ahead of slidebox. I guess slidebox is trying to cope with a lot of different use cases so it's not as well optimised as an image carousel. |
Is there any working solution for lazy loading of slides? |
Any update on this? |
Test is in test/html/slidebox.html
Benefits of refactor:
TODO