fix(item-options): rtl better suport #11188
Conversation
for sliding items *Note:* need tests
src/components/item/item-options.ts
Outdated
|
|
||
| /** | ||
| * @hidden | ||
| */ | ||
| getSides(): number { | ||
| if (isPresent(this.side) && this.side === 'left') { | ||
| if (isPresent(this.side) && this.side === 'left' && !_plt.isRTL()) { |
There was a problem hiding this comment.
This misses 'right' and ltr (I get that you set it as default, just a tiny bit odd. Great work though.
What do you think of?
if (isPresent(this.side))
switch(this.side) {
case 'left':
return _plt.isRTL()? ITEM_SIDE_FLAG_RIGHT: ITEM_SIDE_FLAG_LEFT;
case 'right':
return _plt.isRTL()? ITEM_SIDE_FLAG_LEFT: ITEM_SIDE_FLAG_RIGHT;
}
return ITEM_SIDE_FLAG_RIGHT;
|
Can you review now, @AmitMY? |
|
This looks much better, nice job! Note: If #11233 gets merged, the sliding item will also need to subscribe to the @brandyscarney Anything else here? |
src/components/item/item-options.ts
Outdated
| case 'right': | ||
| return ITEM_SIDE_FLAG_RIGHT; | ||
| case 'start': | ||
| return _plt.isRTL() ? ITEM_SIDE_FLAG_RIGHT : ITEM_SIDE_FLAG_LEFT; |
There was a problem hiding this comment.
This should be this._plt.isRTL()
src/components/item/item-options.ts
Outdated
| case 'start': | ||
| return _plt.isRTL() ? ITEM_SIDE_FLAG_RIGHT : ITEM_SIDE_FLAG_LEFT; | ||
| case 'end': | ||
| return _plt.isRTL() ? ITEM_SIDE_FLAG_LEFT : ITEM_SIDE_FLAG_RIGHT; |
src/components/item/item-options.ts
Outdated
| if (isPresent(this.side) && this.side === 'left') { | ||
| return ITEM_SIDE_FLAG_LEFT; | ||
| if (isPresent(this.side)) { | ||
| switch(this.side) { |
There was a problem hiding this comment.
Can you add whitespace to make it: switch (this.side) (it is giving me linter errors)
|
Thanks @John-Luke! Do the latest changes look good to you @AmitMY? :) |
|
@brandyscarney It now looks even better :) |
|
Merged, thanks! 😄 |
Short description of what this resolves:
fix the side of item-options component when rtl
Ionic Version: 1.x / 2.x / 3.x
Fixes partially: #11115