Zhylka/legacy chips rewriting #2655
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
humanmantis
requested changes
Dec 1, 2024
| @@ -60,7 +60,7 @@ | |||
| >{{ 'FORMS.PLACEHOLDERS.KEYWORDS_START' | translate }} {{ validationConstants.MAX_KEYWORDS_LENGTH }} | |||
| {{ 'FORMS.PLACEHOLDERS.KEYWORDS_END' | translate }}</span | |||
| > | |||
| <mat-chip-list #chipList> | |||
| <mat-chip-grid #chipGrid> | |||
There was a problem hiding this comment.
It is better to use mat-chip-grid together with the input to follow the correct interaction pattern
| @@ -34,8 +34,9 @@ export class ChildFormComponent implements OnInit, OnDestroy { | |||
|
|
|||
| @ViewChild('select') | |||
| private select: MatSelect; | |||
| @ViewChild('chipList') | |||
| private chipList: MatChipList; | |||
|
|
|||
|
I am not sure if ngModel needs to be set on mat-chip-grid (as shown in the docs), because we use reactive forms. So, I removed ngModel from the input and fixed the issue where the input remained disabled after adding 5 elements and then removing one. |
humanmantis
requested changes
Dec 2, 2024
| <input | ||
| [ngClass]="{ 'keywords-input': !keyWord && keyWords.length < 1 }" | ||
| [ngClass]="{ 'keywords-input': !keyWordsCtrl.value && keyWords$.value.length < 1 }" |
There was a problem hiding this comment.
remove extra whitespace after { and before }
| public keyWord: string; | ||
|
|
||
| public disabilityOptionRadioBtn: FormControl = new FormControl(false); | ||
| public disabledKeyWordsInput = false; | ||
|
|
||
| public separatorKeysCodes = [13, 188]; // Enter and Comma |
There was a problem hiding this comment.
Import keycodes from @angular/cdk/keycodes
| @@ -74,18 +75,29 @@ export class CreateDescriptionFormComponent implements OnInit, OnDestroy { | |||
|
|
|||
| this.passDescriptionFormGroup.emit(this.DescriptionFormGroup); | |||
| this.keyWordsListener(); | |||
|
|
|||
| this.keyWords$.pipe(takeUntil(this.destroy$)).subscribe((keyWords) => { | |||
| this.disabledKeyWordsInput = keyWords.length >= 5; | |||
There was a problem hiding this comment.
create a private read-only property for max keywords instead of a magic number
| @@ -40,12 +40,13 @@ export class CreateDescriptionFormComponent implements OnInit, OnDestroy { | |||
| public SectionItemsFormArray = new FormArray([]); | |||
| public keyWordsCtrl: FormControl = new FormControl('', Validators.required); | |||
|
|
|||
| public keyWords: string[] = []; | |||
| public keyWords$: BehaviorSubject<string[]> = new BehaviorSubject<string[]>([]); | |||
There was a problem hiding this comment.
Why do we need a subject in this case?
We can simply do checks inside the method or create a separate method to switch the input state
| if (this.keyWords.length) { | ||
| this.DescriptionFormGroup.get('keyWords').setValue([...this.keyWords]); | ||
| const keyWords = this.keyWords$.value; | ||
| const index = keyWords.indexOf(word); |
|
humanmantis
approved these changes
Dec 3, 2024
Liuboff
added a commit
that referenced
this pull request
Dec 5, 2024
* Liuboff/Replacing legacy snack bar * Liuboff/Replacing legacy snack bar * Zhylka/legacy chips rewriting (#2655) * chip-list replaced by chip-set/chip-listbox * all chip-lists replaced by chip-set * replaced list-set by list-grid in create-description-form for passing tests * added test for removing chips when changing language * removed ngModel and fixed input behavior after adding 5 elements * removed behavior subject, added function to change keyword input state, fixed remarks * changed cdk version * changed based on PR recommendations * Liuboff/Replacing legacy snack bar * changed based on PR recommendations * fix tests according PR recomendations * Replace > to previous line * fix tests according PR recomendations * Replace > to previous line --------- Co-authored-by: Liubov Zatsepina <liubov_zatsepina@epam.com> Co-authored-by: MaxZhylka <132847452+MaxZhylka@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaced MatLegacyChipModule, replaced all chip-lists by chip-set. In the CreateDescriptionForm component, specifically updated mat-chip-list to mat-chip-grid to resolve test failures