[Feature] Programmatic navigation between pages #7
Conversation
… set by consumers in case they already have an instance of it in their app
|
|
||
| private get _animateLoadPage(): boolean { | ||
| const current: boolean = this._animateLoadPageValue; | ||
| this._animateLoadPageValue = false; |
There was a problem hiding this comment.
This is a really "hidden" thing and it is not very intuitive that the flag will be lowered once it is returned. How about we change this to a simple property and just raise the flag before navigation and the lower it manually?
| @@ -27,6 +32,9 @@ export class ImageSwipe extends ScrollView { | |||
|
|
|||
| public ios: any; /* UIScrollView */ | |||
| public android: any; /* android.support.v4.view.ViewPager */ | |||
|
|
|||
| public nextPage(): void; | |||
There was a problem hiding this comment.
How about instead of having just next and previous page navigation we change this to public scrollToPage(pageNumber: number, animated?: boolean)? This will allow much broader cases than just +1 and -1 for pages. Also will allow the user to control whether this is animated or not.
There was a problem hiding this comment.
Not sure if that works nicely with the way the prev and next views are loaded in iOS. I can give that a try.
| @@ -67,6 +67,14 @@ export class ImageSwipe extends ImageSwipeBase { | |||
| // Coerce selected index after we have set items to native view. | |||
| pageNumberProperty.coerce(this); | |||
| } | |||
|
|
|||
| public nextPage(): void { | |||
There was a problem hiding this comment.
For animation to occur on android shouldn't this line: https://github.com/Buuhuu/nativescript-image-swipe/blob/dd03eb2f5a2cbf59e899945f0c46d949d20c2f7e/image-swipe.android.ts#L61 be changed to this.nativeView.setCurrentItem(value, true) (so basically a similar approach as on iOS)?
There was a problem hiding this comment.
Works quite well even without specifying that explicitly.
| @@ -17,6 +17,11 @@ import { EventData } from "data/observable"; | |||
| import { CoercibleProperty, Property } from "ui/core/view"; | |||
| import { ItemsSource } from "ui/list-picker"; | |||
| import { ScrollView } from "ui/scroll-view"; | |||
| import { Cache } from "ui/image-cache"; | |||
|
|
|||
| export class ImageSwipeBase { | |||
There was a problem hiding this comment.
Exporting the base class is a bad thing. Since we are extending the base class the cache should be visible on the main ImageSwipe class. But seems there is a bug in {N} that does not copy static objects from the base class. So I suggest for now to remove this and have it available as part of the main class later once the bug is fixed. Or for now we can add the property in the platform specific files and change its name to not be prefixed with _ since now it will be available for outside usage.
There was a problem hiding this comment.
Ok, +1 for the later approach. I think its more a typescript issue then {N} one.
There was a problem hiding this comment.
Here are the issues I've submitted for both android and ios runtime:
NativeScript/android#783
NativeScript/ios-jsc#773
This implements programmatic navigation between pages. It's useful in case the app requires to have buttons to navigate between the images beside the swipe gesture.