-
Notifications
You must be signed in to change notification settings - Fork 13.5k
feat(item-reorder): add side support #11639
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
Conversation
@@ -276,6 +278,7 @@ import { ItemReorder } from './item-reorder'; | |||
template: | |||
'<ng-content select="[item-start],[item-left],ion-checkbox:not([item-end]):not([item-right])"></ng-content>' + | |||
'<div class="item-inner">' + | |||
'<ion-reorder *ngIf="_reorderSide == \'start\'" class="reorder-start"></ion-reorder>' + |
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.
we don't need reorder-start, reorder-end and have, ion-reorder
duplicated, we can just use CSS.
order: -1
can make the trick
* @param isRTL whether the application dir is rtl | ||
* @param defaultStart whether the default side is start | ||
*/ | ||
export function isStartSide(side: Side, isRTL: boolean, defaultStart: boolean = true): boolean { |
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.
can't we reuse isRightSide(side: Side, isRTL: boolean, defaultRight: boolean = false)?
@@ -158,6 +158,11 @@ export class ItemReorder implements ItemReorderGestureDelegate { | |||
*/ | |||
@Output() ionItemReorder: EventEmitter<ReorderIndexes> = new EventEmitter<ReorderIndexes>(); | |||
|
|||
/** | |||
* @input {string} Which side of the view the menu should be placed. Default `"start"`. |
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.
fix comment!
@@ -16,12 +16,18 @@ ion-reorder { | |||
font-size: 1.7em; | |||
opacity: .25; | |||
|
|||
transform: translate3d(160%, 0, 0); |
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 think it is better to revert and have right side by default, then add: "reorder-end" dynamically
transition: transform 140ms ease-in; | ||
|
||
pointer-events: all; | ||
touch-action: manipulation; | ||
|
||
&.reorder-end { |
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.
we prefer to avoid using nested sass
@Optional() reorder: ItemReorder | ||
) { | ||
super(config, elementRef, renderer, 'item'); | ||
|
||
this._setName(elementRef); | ||
this._hasReorder = !!reorder; | ||
this._reorderSide = reorder ? (isStartSide(reorder.side, plt.isRTL) ? 'start' : 'end') : null; |
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.
we don't need this anymore, also, it is not a good idea to calculate this here, since side can change dynamically?
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.
also, revert _hasReorder, (we don't need two <ion-reorder>
)
@@ -134,7 +135,7 @@ export class ItemReorderGesture { | |||
} | |||
|
|||
private itemForCoord(coord: PointerCoordinates): HTMLElement { | |||
const sideOffset = this.plt.isRTL ? 100 : -100; | |||
const sideOffset = isRightSide(this.reorderList.side, this.plt.isRTL) ? -100 : 100; |
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.
could we cache some "_reorderSide" as a boolean (probably renamed to (_isRightSide) that is calculated in ngOnInit()?
Short description of what this resolves:
A user needs this feature
Video:
https://youtu.be/vlRo4PMnhEc
Changes proposed in this pull request:
ion-item
Of course, this is RTL supported. Except for the
transform
, which will be fixed in an upcoming PR so I did not want to make custom.Ionic Version: 3.x
Fixes: #11637