-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(tabs): add swipe events #6294
Changes from 11 commits
9c7f20b
7f2ff6f
cb40cc8
aaa6cef
9c84af8
c2ce180
43282e7
33d584e
897275a
75ffcc2
5557b80
36d422c
5928c20
343a8dd
b4d5bee
031672f
0733532
718ec6d
eecd2bd
6875bba
e8aed99
63c1ab5
8d79f7c
79e2e11
e5a7d3e
c9ee561
5b801b0
be263e6
c4a61a8
a35a9a7
9b22614
ca91b01
59ef6b4
acb5525
d68f2f0
52c5b4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,10 @@ import { | |
AfterContentInit, | ||
AfterContentChecked, | ||
OnDestroy, | ||
Optional, | ||
} from '@angular/core'; | ||
import {HammerInput, HammerDirection} from '../core'; | ||
import {Directionality} from '@angular/cdk/bidi'; | ||
import {coerceBooleanProperty} from '@angular/cdk/coercion'; | ||
import {map} from '@angular/cdk/rxjs'; | ||
import {Observable} from 'rxjs/Observable'; | ||
|
@@ -143,7 +146,8 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
|
||
constructor(_renderer: Renderer2, | ||
elementRef: ElementRef, | ||
private _changeDetectorRef: ChangeDetectorRef) { | ||
private _changeDetectorRef: ChangeDetectorRef, | ||
@Optional() private _dir: Directionality) { | ||
super(_renderer, elementRef); | ||
this._groupId = nextId++; | ||
} | ||
|
@@ -185,7 +189,7 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
} | ||
} | ||
|
||
ngAfterContentInit() { | ||
ngAfterContentInit(): void { | ||
this._subscribeToTabLabels(); | ||
|
||
// Subscribe to changes in the amount of tabs, in order to be | ||
|
@@ -196,7 +200,7 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
}); | ||
} | ||
|
||
ngOnDestroy() { | ||
ngOnDestroy(): void { | ||
if (this._tabsSubscription) { | ||
this._tabsSubscription.unsubscribe(); | ||
} | ||
|
@@ -214,7 +218,7 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
this._isInitialized = true; | ||
} | ||
|
||
_focusChanged(index: number) { | ||
_focusChanged(index: number): void { | ||
this.focusChange.emit(this._createChangeEvent(index)); | ||
} | ||
|
||
|
@@ -233,7 +237,7 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
* binding to be updated, we need to subscribe to changes in it and trigger change detection | ||
* manually. | ||
*/ | ||
private _subscribeToTabLabels() { | ||
private _subscribeToTabLabels(): void { | ||
if (this._tabLabelSubscription) { | ||
this._tabLabelSubscription.unsubscribe(); | ||
} | ||
|
@@ -276,4 +280,42 @@ export class MdTabGroup extends _MdTabGroupMixinBase implements AfterContentInit | |
this._tabBodyWrapperHeight = this._tabBodyWrapper.nativeElement.clientHeight; | ||
this._renderer.setStyle(this._tabBodyWrapper.nativeElement, 'height', ''); | ||
} | ||
|
||
/** Body content was swiped left/right */ | ||
_bodyContentSwiped(event: HammerInput): void { | ||
if (this.selectedIndex === null) { | ||
this.selectedIndex = 0; | ||
} | ||
|
||
if (event.direction === HammerDirection.Left || event.direction === HammerDirection.Right) { | ||
let direction = event.direction; | ||
if (this._dir.value === 'rtl') { | ||
direction = | ||
direction === HammerDirection.Left ? HammerDirection.Right : HammerDirection.Left; | ||
} | ||
|
||
if (this.selectedIndex !== 0 && direction === HammerDirection.Right) { | ||
this._setActiveItemByIndex(this.selectedIndex - 1, -1); | ||
} else if (this.selectedIndex < this._tabs.length && direction === HammerDirection.Left) { | ||
this._setActiveItemByIndex(this.selectedIndex + 1, 1); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Sets the active item to the first enabled item starting at the index specified. If the | ||
* item is disabled, it will move in the fallbackDelta direction until it either | ||
* finds an enabled item or encounters the end of the list. | ||
*/ | ||
private _setActiveItemByIndex(index: number, | ||
fallbackDelta: number, | ||
items = this._tabs.toArray()): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why include the items array as a param? I don't see it passed in by anyone There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was a pattern from another component that I used that @jelbourn referred me to. Do you think it should just be an variable defined in the function body? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can remove it if the param is never passed |
||
if (!items[index]) { return; } | ||
while (items[index].disabled) { | ||
index += fallbackDelta; | ||
if (!items[index]) { return; } | ||
} | ||
this.selectedIndex = index; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment for why this needs to be an inline style? Also, since this is static, you don't need a binding; just
style="touch-action: pan-y"
is good.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like the comment to be a HTML inline comment?
This actually doesn't work if you do it static like you suggested. I had tried that at first implementation.
Reference: hammerjs/hammer.js#1014
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent a bit too much time tracking this down.
touch-action: none
is coming from our materialGestureConfig
; adding the swipe gesture always sets the touch action topan-y
+pan-x
(equivalent tonone
).The "correct" fix for this would be to change how our gesture config add recognizers for certain events, but that's more complicated than we want to do right now. I filed #6484 to track this. Can you make your comment something like
<!-- touch-action workaround for https://github.com/angular/material2/issues/6484 -->
Comment can go above the element (I don't think a comment inside of a tag is valid html). HTML comments are stripped when packaging the release