Add tag component#30
Conversation
MaximDomanov
changed the title
feerzlay
left a comment
feerzlay
left a comment
There was a problem hiding this comment.
И помимо замечаний, еще попросил бы интерактивным ребейзом поправить текст коммита в соответствии с CONTRIBUTING, а именно:
- use the imperative, present tense: "change" not "changed" nor "changes"
- don't capitalize the first letter
| excludeComponentDeclaration: true | ||
| }); | ||
|
|
||
| expect(component.fixture.nativeElement.querySelector('.es-tag').style.backgroundColor).toBe( |
There was a problem hiding this comment.
Лучше сделать getByTestId, это правильнее с точки зрения идеологии этой либы.
getByText > getByTestId > querySelector
| expect(component.fixture.nativeElement.querySelector('.es-tag').style.backgroundColor).toBe( | ||
| 'rgb(0, 0, 0)' | ||
| ); | ||
| expect(component.fixture.nativeElement.querySelector('.es-tag__text').style.color).toBe( |
There was a problem hiding this comment.
У нас стоит расшрение https://github.com/testing-library/jest-dom и в нем помимо прочего есть toHaveStyle
|
|
||
| &__icon { | ||
| font-size: 14px; | ||
| height: 14px !important; |
There was a problem hiding this comment.
Надо бы линтером запредить important :) Можно повышая специфичность сделать &.mat-icon {}
| { | ||
| "lib": { | ||
| "entryFile": "public-api.ts", | ||
| "cssUrl": "inline" |
There was a problem hiding this comment.
Эта строка тут в принципе не нужна
| }} | ||
| </Story> | ||
| </Preview> | ||
|
|
There was a problem hiding this comment.
Нужен пример с svg иконкой
| @Input() icon?: string; | ||
|
|
||
| /** | ||
| * Override the icon displayed before the text. |
There was a problem hiding this comment.
Может для единообразия сделать "The svg icon displayed before the text."?
| @@ -0,0 +1,11 @@ | |||
| import { NgModule } from '@angular/core'; | |||
| import { CommonModule } from '@angular/common'; | |||
| import { ESTagComponent } from './tag.component'; | |||
There was a problem hiding this comment.
Мелкая стилистическая придирка, локальный импорт вниз переместить :)
| > | ||
| {{ currentIcon.icon || '' }} | ||
| </mat-icon> | ||
| <div class="es-tag__text mat-small" [ngStyle]="{ color: textColor }"> |
There was a problem hiding this comment.
Лучше mat-caption, это правильное название по материалу
| */ | ||
| @Input() svgIcon?: string; | ||
|
|
||
| get currentIcon() { |
There was a problem hiding this comment.
Добавить, я в alert забыл про это
/**
* @internal
* @ignore
*/
| </Story> | ||
| </Preview> | ||
|
|
||
| We can customize element |
There was a problem hiding this comment.
Точки в конце предложений, для единообразия документации.
2 participants
No description provided.