Add badge component#50
Conversation
|
|
||
| public ngOnInit() { | ||
| this._elementRef.nativeElement.style.setProperty('--right', `${-this.right + 1 + `px`}`); | ||
| this._elementRef.nativeElement.style.setProperty('--bottom', `${-this.bottom + 1 + `px`}`); |
AndreyMorozow
force-pushed
the
feature/32-badge
branch
from
e2489db to
1d05451
Compare
AndreyMorozow
force-pushed
the
feature/28-avatar
branch
from
f12e4cb to
38aa2a4
Compare
AndreyMorozow
force-pushed
the
feature/32-badge
branch
from
1d05451 to
49b7915
Compare
| @@ -4,6 +4,7 @@ | |||
| @import './ui/paginator/paginator.component.theme'; | |||
There was a problem hiding this comment.
Вообще этого файла в девелорпе уже нет, может в ветке аватара затисался?
| </div> | ||
| </div> | ||
| <div #childElement> | ||
| <ng-content select="[es-role=child]"></ng-content> |
There was a problem hiding this comment.
Это можно просто без select оставить
| <div class="es-badge__content" | ||
| [ngStyle]="stylesObject()" | ||
| > | ||
| <ng-content select="[es-role=icon]"></ng-content> |
There was a problem hiding this comment.
Лучше без префикса es-, просто <ng-content select="[role=icon]"></ng-content>
| [ngStyle]="stylesObject()" | ||
| > | ||
| <ng-content select="[es-role=icon]"></ng-content> | ||
| <div data-testid="badge" class="es-caption es-badge__count"> |
There was a problem hiding this comment.
es-badge__count лишний видимо? А то в стилях не используется
| offsetHorizontal?: number; | ||
| } | ||
|
|
||
| export type ESBadgePositionVariant = |
There was a problem hiding this comment.
Не нравится это дублирование. Можно убрать этот union, ESBadgePositions переименовать в ESBadgePosition, а когда нужно получить union то делать так - position?: ${ESBadgePosition};.
| import { NgModule } from '@angular/core'; | ||
| import { CommonModule } from '@angular/common'; | ||
|
|
||
| import { MatIconModule } from '@angular/material/icon'; |
There was a problem hiding this comment.
Ему не нужно импортировать этот модуль, не использует же напрямую
| } | ||
| this.setOffsets(); | ||
| this.setStyles(); | ||
| this.changeDetector.markForCheck(); |
There was a problem hiding this comment.
Вообще markForCheck иногда может долго думать и тогда лучше detectChanges делать
| switch (this.position) { | ||
| case this.badgePosition.AboveAfter: { | ||
| this.positions = { 'top.px': 0, 'right.px': -1 }; | ||
| this.offsetVertical = childElement - transparent; |
There was a problem hiding this comment.
А почему оно влияет на инпуты? Эти offset должны просто к прибавляться к / вычитаться из позиции, этакая небольшая коррекция.
А если нужны для позиции для маски то можно прямо тут их задавать, не сохраняя в промежуточную переменную
| * @internal | ||
| * @ignore | ||
| */ | ||
| public stylesObject() { |
There was a problem hiding this comment.
Можно ли это хранить в поле а не каждый раз в темплейте деграть функцию?
There was a problem hiding this comment.
Да нет, обычно используют функции
https://blog.angular-university.io/angular-ngclass-ngstyle/
https://ultimatecourses.com/blog/using-ngstyle-in-angular-for-dynamic-styling
AndreyMorozow
force-pushed
the
feature/28-avatar
branch
from
9755329 to
fbf8bfc
Compare
AndreyMorozow
force-pushed
the
feature/32-badge
branch
from
26897e0 to
b0da3a4
Compare
feerzlay
force-pushed
the
feature/28-avatar
branch
from
fbf8bfc to
0639efb
Compare
| public ngAfterContentInit() { | ||
| this.setAvatarsIndex(this.avatars); | ||
| this.avatars.changes.pipe(takeUntil(this.destroyed$)).subscribe((avatars) => { | ||
| if (avatars) { |
| * @internal | ||
| * @ignore | ||
| */ | ||
| public destroyed$ = new Subject(); |
There was a problem hiding this comment.
Может нам использовать стороннюю библиотеку https://github.com/ngneat/until-destroy для этих случаев? Мне кажется, не лучший вариант писать такой Subject каждый раз
There was a problem hiding this comment.
А как по мне зачем тащить либу, которая возможно на проекте использоваться и не будет. Да и такой вариант с Subject не сильно больше кода требует.
| return this._typography; | ||
| } | ||
| public set typography(value: string) { | ||
| this._typography = value || 'es-subtitle-2'; |
There was a problem hiding this comment.
Какое-то неочевидное поведение. Я думаю, лучше сделать в get return this._typography || 'es-subtitle-2'
| * Defines badge background color. | ||
| */ | ||
| @Input() | ||
| public get color(): string { |
There was a problem hiding this comment.
Думаю, здесь можно обойтись без set/get
AndreyMorozow
force-pushed
the
feature/32-badge
branch
from
b0da3a4 to
08fb677
Compare
4 participants
https://is.elonsoft.ru/projects/107/features/32