Skip to content

[Feature] Programmatic navigation between pages #7

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.

Sign up for GitHub

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

Open
buuhuu wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[Feature] Programmatic navigation between pages #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4617185
- expose the static _imageCache member of ImageSwipeBase so it can be…
Jun 18, 2017
dd03eb2
- added nextPage() prevPage() to programmaticaly navigate between pages
Jun 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion image-swipe-common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ImageSwipe as ImageSwipeDefinition } from ".";

export * from "ui/scroll-view";

export class ImageSwipeBase extends ScrollView implements ImageSwipeDefinition {
export abstract class ImageSwipeBase extends ScrollView implements ImageSwipeDefinition {
public static pageChangedEvent: string = "pageChanged";

public static _imageCache: Cache;
Expand All @@ -42,6 +42,14 @@ export class ImageSwipeBase extends ScrollView implements ImageSwipeDefinition {
public _getDataItem(index: number): any {
return this.isItemsSourceIn ? (this.items as ItemsSource).getItem(index) : this.items[index];
}

public nextPage(): void {
this.pageNumber ++;
}

public prevPage(): void {
this.pageNumber --;
}
}

export const pageNumberProperty = new CoercibleProperty<ImageSwipeBase, number>({
Expand Down
8 changes: 8 additions & 0 deletions image-swipe.android.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Owner

@PeterStaev PeterStaev Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

Copy link
Contributor Author

@buuhuu buuhuu Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works quite well even without specifying that explicitly.

this.pageNumber ++;
}

public prevPage(): void {
this.pageNumber --;
}
}

@Interfaces([android.support.v4.view.ViewPager.OnPageChangeListener])
Expand Down
8 changes: 8 additions & 0 deletions image-swipe.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Owner

@PeterStaev PeterStaev Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@buuhuu buuhuu Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, +1 for the later approach. I think its more a typescript issue then {N} one.

Copy link
Owner

@PeterStaev PeterStaev Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are the issues I've submitted for both android and ios runtime:
NativeScript/android#783
NativeScript/ios-jsc#773

public static _imageCache: Cache;
}

export class ImageSwipe extends ScrollView {
public static pageChangedEvent: string;
Expand All @@ -27,6 +32,9 @@ export class ImageSwipe extends ScrollView {

public ios: any; /* UIScrollView */
public android: any; /* android.support.v4.view.ViewPager */

public nextPage(): void;
Copy link
Owner

@PeterStaev PeterStaev Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@buuhuu buuhuu Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if that works nicely with the way the prev and next views are loaded in iOS. I can give that a try.

public prevPage(): void;
}

export interface PageChangeEventData extends EventData {
Expand Down
27 changes: 26 additions & 1 deletion image-swipe.ios.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class ImageSwipe extends ImageSwipeBase {

private _views: Array<{ view: UIView; imageView: UIImageView; zoomDelegate: UIScrollViewZoomDelegateImpl }>;
private _delegate: UIScrollViewPagedDelegate;
private _animateLoadPageValue: boolean = false;

constructor() {
super();
Expand Down Expand Up @@ -85,7 +86,7 @@ export class ImageSwipe extends ImageSwipeBase {
const pageWidth = scrollView.frame.size.width;

if (!this.isScrollingIn) {
scrollView.contentOffset = CGPointMake(value * pageWidth, 0);
scrollView.setContentOffsetAnimated(CGPointMake(value * pageWidth, 0), this._animateLoadPage);
}

for (let loop = 0; loop < value - 1; loop++) {
Expand Down Expand Up @@ -135,6 +136,26 @@ export class ImageSwipe extends ImageSwipeBase {
imageView.frame = contentsFrame;
}

public nextPage(): void {
this._animateLoadPage = true;
super.nextPage();
}

public prevPage(): void {
this._animateLoadPage = true;
super.prevPage();
}

private get _animateLoadPage(): boolean {
const current: boolean = this._animateLoadPageValue;
this._animateLoadPageValue = false;
Copy link
Owner

@PeterStaev PeterStaev Jun 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

return current;
}

private set _animateLoadPage(value: boolean) {
this._animateLoadPageValue = value;
}

private _resizeNativeViews(page: number) {
if (page < 0 || page >= this.items.length) { // Outside Bounds
return;
Expand Down Expand Up @@ -300,6 +321,10 @@ class UIScrollViewPagedDelegate extends NSObject implements UIScrollViewDelegate
this._owner.get().isScrollingIn = true;
}

public scrollViewDidEndScrollingAnimation(scrollView: UIScrollView) {
this._owner.get().isScrollingIn = false;
}

public scrollViewDidEndDecelerating(scrollView: UIScrollView) {
const pageWidth = scrollView.frame.size.width;
const owner = this._owner.get();
Expand Down