feat(form-field): merge original and experimental modules (#DS-3463) #869
Conversation
|
Visit the preview URL for this PR (updated for commit 043e717): https://koobiq-next--prs-869-r5r1pj3g.web.app (expires Sat, 05 Jul 2025 08:44:16 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c |
|
|
||
| private readonly reactivePasswordHint = contentChildren(KbqReactivePasswordHint); |
There was a problem hiding this comment.
где-то contentChildren, где-то @ContentChildren, для читабельности надо к единому виду привести
There was a problem hiding this comment.
только в новом коде использую сигналы, чтобы не было ломающих изменений или изменяющих api
There was a problem hiding this comment.
Из этого исхожу:
Mixing different style conventions in a single file creates more confusion than diverging from the recommendations in this guide.
https://angular.dev/style-guide#when-in-doubt-prefer-consistency
There was a problem hiding this comment.
Pull Request Overview
This PR merges the original and experimental form-field modules and updates the validation provider usage across multiple modules, replacing deprecated KBQ_VALIDATION imports with the new kbqDisableLegacyValidationDirectiveProvider. In addition, various components, templates and styles have been refactored to use Angular’s standalone API and to improve consistency in selector and content projection patterns.
Reviewed Changes
Copilot reviewed 79 out of 79 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/public_api_guard/components/form-field.api.md | Updated public API comments and exports for form-field components. |
| packages/components/form-field/validate.directive.ts | Refactored dependency injection to include ElementRef and updated host class assignments. |
| packages/components/form-field/form-field.html | Combined content projection selectors for password toggle, stepper and suffix elements. |
| packages/components/core/validation/validation.ts | Replaced use of KBQ_VALIDATION with kbqDisableLegacyValidationDirectiveProvider in provider factories. |
| packages/components-experimental/form-field/form-field.module.ts | Marked experimental module as deprecated and aligned component imports with the original package. |
| Other example and test files | Updated imports to reflect changes in validation provider and form-field module structure. |
Comments suppressed due to low confidence (3)
packages/components/form-field/validate.directive.ts:101
- It would be helpful to add an inline comment explaining why the directive adds CSS classes to both the parent form field element and its own ElementRef. This clarification would improve maintainability and assist other developers in understanding the rationale behind the dual assignment.
this.parentFormField?.elementRef.nativeElement.classList.add('kbq-form-field_has-validate-directive');
packages/components/form-field/form-field.html:26
- [nitpick] The merging of content projection selectors for password-toggle, stepper and kbqSuffix is a significant design decision. Please confirm that this consolidated projection is intentional and consider adding documentation to clarify how these projected elements are intended to interact to avoid potential layout or styling issues.
<ng-content select="kbq-password-toggle, kbq-stepper, [kbqSuffix]" />
packages/components-experimental/form-field/form-field.module.ts:27
- [nitpick] Consider adding clear migration instructions or a reference to the new documentation in this deprecation notice, so that developers know how to transition from the experimental module to the original package.
*/
| template: ` | ||
| <kbq-form-field> | ||
| <input [formControl]="formControl" placeholder="Password" kbqInputPassword /> | ||
|
|
||
| <kbq-password-toggle /> | ||
|
|
||
| <kbq-password-hint [hasError]="formControl.hasError('minLength')"> | ||
| <kbq-reactive-password-hint [hasError]="formControl.hasError('minLength')"> |
There was a problem hiding this comment.
Цвет иконки меняется при блюре. Выглядит странно, когда иконка с галкой, а цвет серый или красный
Screen.Recording.2025-06-30.at.15.04.33.mov
| }) | ||
| export class KbqLabel { | ||
| /** Unique id for the label. */ | ||
| @Input() id: string = `kbq-label-${nextUniqueId++}`; |
There was a problem hiding this comment.
id не должен быть инпутом. Это касается всех компонентов
| // conditionally display the prefix/suffix containers based on whether there is projected content. | ||
| merge(this.prefix.changes, this.suffix.changes) | ||
| .pipe(takeUntilDestroyed(this.destroyRef)) | ||
| .subscribe(() => { |
There was a problem hiding this comment.
в одну строку без скобок
| canCleanerClearByEsc: boolean = true; | ||
|
|
||
| private readonly destroyRef = inject(DestroyRef); | ||
| /** The form field control. */ | ||
| get control(): KbqFormFieldControl<unknown> { |
There was a problem hiding this comment.
Непонятен смысл этого геттера. По идее в afterContentInit нужно чекать наличие контрола и если его нет бросать ошибку. Геттер можно убрать а _control переименовать
| '[class.kbq-form-field_invalid]': 'invalid', | ||
| // legacy class for backward compatibility | ||
| '[class.kbq-disabled]': 'disabled', | ||
| '[class.kbq-form-field_disabled]': 'disabled', |
There was a problem hiding this comment.
зачем два disabled класса ?
| import { KbqStepper } from './stepper'; | ||
| import { KbqSuffix } from './suffix'; | ||
|
|
||
| let nextUniqueId = 0; | ||
| /** @docs-private */ | ||
| export const getKbqFormFieldMissingControlError = (): Error => { |
There was a problem hiding this comment.
с какой целью function заменен на const ?
No description provided.