Lecture 11 dz #118
Lecture 11 dz #118
Conversation
| @@ -2,7 +2,7 @@ | |||
| <mat-form-field appearance="fill"> | |||
| <mat-label>Search</mat-label> | |||
| <!-- <input matInput [formControl]="control" /> --> | |||
| <input matInput formControlName="name" /> | |||
| <input matInput formControlName="name" value="filter?.name || ''" /> | |||
There was a problem hiding this comment.
А зачем мы так делаем? Почему не в FormControl устанавливается значение?
Не очень хорошая практика, когда подключается механизм фреймворка и параллельно ему используется механизм нативный - может возикнуть не ожидаемое поведение. Например, дирректива отаботает не корректно
| brands: this.formBuilder.array<FormControl<boolean>>([]), | ||
| priceRange: this.formBuilder.group({ | ||
| min: 0, | ||
| max: 999999, | ||
| min: ProductsFilterConstants.min, |
There was a problem hiding this comment.
Почему решил именно class использовать для этого? Почему не 2 const?
| private updateFilter() { | ||
| this.filterForm.controls.name?.setValue(this.queryParams?.name || ''); | ||
|
|
||
| this.brandsFromQueryParams = |
There was a problem hiding this comment.
На самом деле свойство brandsFromQueryParams мне кажется излишним: данное свойство нужно лишь для извлечения данных хранящихся в инпуте и используется только в 1 методе - тогда почему бы нам в методе updateBrandsControl не извлечь данные из Input и ипользовать их на месте, не сохраняя в свойство? Так получится и прозрачней и надежней - ведь сейчас, если brands попадут сразу в инпут (конечно вряд ли такое В НАШЕМ СЛУЧАЕ возможно, но в целом релевантный кейс), то отработает ngOnCneges где произойдет обновление контрола и только потом отработает updateFilter из ngOnInit (P.S. Сейчас это работает, потому что ngOnChanges с brands отрабатывает с задержкой из-за асинхронности запроса)
| priceMin: filter.priceRange.min, | ||
| priceMax: filter.priceRange.max, | ||
| }; | ||
| this.router.navigate(['products-list'], { |
There was a problem hiding this comment.
А это будет работать для пути products-list/:id? Кажется, что произойдет непредвиденная навигация
Попробуй воспользоваться относительной навигацией по пустому сегменту. Думаю, получится прям то, что надо👍
| } | ||
|
|
||
| private reloadAfterChangeQueryParams(filter: IProductsFilter) { | ||
| this.productsQueryParams = { |
There was a problem hiding this comment.
А нужно ли нам тут сохранять свойство? Ведь в конце концов у нас произойдет навигациция с установкой query параметров -> выстрелит поток -> обновит значение в подписке. Т.е. по факту, сейчас, при изменении фильтра - значение обновляется 2 раза одними и теми же данными
| } | ||
|
|
||
| private listenQueryParamMap() { | ||
| this.activatedRoute.queryParamMap.pipe(takeUntil(this.destroy$)).subscribe(params => { |
There was a problem hiding this comment.
На вскидку - сейчас проблем нет, но гипотетически может быть проблема с OnPush и его флагом Dirty, т.к. мы в подписке не дергаем markForCheck. Нужно его добавить, или, я бы даже порекомендовал отказаться от свойства productsQueryParams и построить поток основанный на параметрах ActivatedRoute и подписаться на него через AsyncPipe в шаблоне. Это будет проще и чище, на мой взгляд (уйдет отписка и сервис, а так же не будет проблемы с markForCheck)
No description provided.