fix(tabs): tabs should update when tabs are added or removed #2014
Conversation
googlebot
added
the
cla: yes
|
|
||
| export type MdTabBodyOriginState = 'left' | 'right'; | ||
|
|
||
| @Component({ |
There was a problem hiding this comment.
Add class comment explaining what this component is for and that it's internal-only.
| state('left-origin-center', style({transform: 'translate3d(0, 0, 0)'})), | ||
| state('right-origin-center', style({transform: 'translate3d(0, 0, 0)'})), | ||
| state('center', style({transform: 'translate3d(0, 0, 0)'})), | ||
| state('right', style({transform: 'translate3d(100%, 0, 0)'})), |
There was a problem hiding this comment.
Can you add a comment for what the different states represent and maybe a high-level explanation of the animations?
| ]) | ||
| ], | ||
| host: { | ||
| 'md-tab-body-active': "'this._position == 'center'" |
There was a problem hiding this comment.
Is this meant to be '[class.md-tab-body-active]'?
| this._origin = 'left'; | ||
| } else { | ||
| this._origin = 'right'; | ||
| } |
There was a problem hiding this comment.
More readable to add a variable for dir:
let dir = this._getLayoutDirection();
if ((dir == 'ltr' && origin <= 0) || (dir == 'rtl' && origin > 0)) {|
|
||
| /** The shifted index position of the tab body, where zero represents the active center tab. */ | ||
| _position: MdTabBodyPositionState; | ||
| @Input('md-tab-body-position') set position(position: number) { |
There was a problem hiding this comment.
You can omit the alias for all of these inputs. Just position, origin, are fine.
In general, only the selector for a directive has the md- prefix, and we should prefer using camelCase properties for binding instead of the dash-case.
andrewseguin
force-pushed
the
tabs-dynamictabs
branch
from
06e5e3b
to
9448117
Compare
|
Added comments to the MdTabBodyPositionState and MdTabBodyOriginState to hopefully clarify how they are used. Let me know if they're still too confusing/abstract. |
andrewseguin
force-pushed
the
tabs-dynamictabs
branch
from
9448117
to
847ebfd
Compare
|
LGTM |
jelbourn
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Closes #1692