Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| ], | ||
| }) | ||
| export class SelectComponent<T = unknown> implements AfterContentChecked, AfterViewChecked, ControlValueAccessor { | ||
| inputId = input.required<string>(); |
There was a problem hiding this comment.
add descriptions like in other components
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
| > | ||
| @if (searchable()) { | ||
| <div class="tedi-select__search-wrapper"> | ||
| @if (!searchTerm() && selectedValues().length && !multiple()) { |
There was a problem hiding this comment.
Maybe extract this to a computed?
There was a problem hiding this comment.
This template is quite large and nested which makes it harder to read and maintain. Maybe extract it into smaller components?
There was a problem hiding this comment.
seems like good idea, but might actually add complexity, as elements are quite tightly coupled (state/signals, KB navigation, angular CDK)
tedi/components/overlay/dropdown/dropdown-item-value/dropdown-item-value.component.scss
Show resolved
Hide resolved
tedi/components/overlay/dropdown/dropdown-item-value/dropdown-item-value.component.scss
Outdated
Show resolved
Hide resolved
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
| flex-wrap: wrap; | ||
| gap: var(--form-field-inner-spacing); | ||
| align-items: center; | ||
| min-height: 1.5em; |
There was a problem hiding this comment.
Is min-height: 1.5em; needed here? If yes, maybe we could use a design token for value instead?
| position: relative; | ||
| flex: 1; | ||
| width: auto; | ||
| min-width: 50px; |
There was a problem hiding this comment.
Maybe we should have a design token, e.g. --input-min-width here?
|
|
||
| .tedi-dropdown-item--selected { | ||
| * { | ||
| color: inherit !important; |
There was a problem hiding this comment.
Is the !imporant here necessary?
| label: { control: "text" }, | ||
| required: { control: "boolean" }, | ||
| placeholder: { control: "text" }, | ||
| state: { control: "radio", options: ["error", "valid", "default"] }, |
There was a problem hiding this comment.
I would add the same default values as in the component to the args as well.
| pointer-events: none; | ||
| } | ||
|
|
||
| &__radio { |
There was a problem hiding this comment.
According to Figma, selected radio should have a blue dot and border and white background.
| @@ -0,0 +1,11 @@ | |||
| <!-- Project custom tedi-dropdown-item-value directly if provided --> | |||
There was a problem hiding this comment.
Are these comments necessary?
| import { InjectionToken, Signal, WritableSignal } from "@angular/core"; | ||
|
|
||
| export interface DropdownApi { | ||
| value: WritableSignal<string | undefined>; |
There was a problem hiding this comment.
Maybe add descriptions here as well?
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
|
Sometime in the relative short past (angular 21), angular team released @angular/aria, which should simplify greatly the development process of combobox/select/etc. Have we thought about using @angular/aria? |
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
4 participants
No description provided.