Skip to content
This repository has been archived by the owner on Oct 1, 2018. It is now read-only.

feat(operators): add searchbar #264

Closed
12 changes: 9 additions & 3 deletions src/app/operators/operators.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@
[fixedInViewport]="true"
class="operator-list-sidenav"
#operatorSidenav>
<form>
<mat-form-field class="full-width-form">
<mat-label>Search</mat-label>
<input matInput value="" [formControl]="searchInput">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that this input is model driven and not template drive, could you move the value initialization of the formControl into the component? IMO is cleaner to not mix template/model driven when possible.

<mat-icon matSuffix>search</mat-icon>
</mat-form-field>
</form>
<mat-nav-list *ngFor="let category of categories"
class="operator-list">
<h3 mat-subheader
class="category-subheader">{{ category }}</h3>
<h3 mat-subheader class="category-subheader" *ngIf="(groupedOperators[category] | filter: searchInput.value).length > 0">{{ category }}</h3>
Copy link
Contributor

@jotatoledo jotatoledo Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wont have a good performance as the # of operators increase, as the filtering is run 2 times. Wouldn't it be better to make the collection of operators mapped to category reactive?

Idea:

Build from Map<category => operator[]> a Map<category => Observable<operator[]>>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I had that idea too, but last time I was simply to supid to implement it that way. But you are definetly right, I will give it another try :D

<a mat-list-item
*ngFor="let operator of groupedOperators[category]"
*ngFor="let operator of groupedOperators[category] | filter: searchInput.value"
[routerLink]="['/operators', operator.name]"
routerLinkActive="active">
{{ operator.name }}
Expand Down
6 changes: 6 additions & 0 deletions src/app/operators/operators.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,9 @@ $subheader-color: #333;
bottom: 10px;
z-index: 4;
}

.full-width-form{
width: 95%;
height: 48px;
padding: 6px;
}
29 changes: 25 additions & 4 deletions src/app/operators/operators.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,33 @@ import {
animate,
transition
} from '@angular/animations';
import { Router, ActivatedRoute } from '@angular/router';
import { BreakpointObserver } from '@angular/cdk/layout';
import { MatSidenav } from '@angular/material';
import { Subscription } from 'rxjs/Subscription';
import { Subject } from 'rxjs/Subject';
import { Observable } from 'rxjs/Observable';
import { filter, takeUntil } from 'rxjs/operators';
import {
filter,
takeUntil,
withLatestFrom,
flatMap,
map,
merge,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are imports here which are unused.

switchAll,
toArray,
concatAll,
reduce,
concat,
tap,
mergeAll,
scan,
debounce,
debounceTime,
distinct
} from 'rxjs/operators';
import { OperatorDoc } from '../../operator-docs/operator.model';
import { OperatorMenuService } from '../core/services/operator-menu.service';
import { FormControl } from '@angular/forms';
import { of } from 'rxjs/observable/of';
import { from } from 'rxjs/observable/from';

const OPERATOR_MENU_GAP_LARGE = 64;
const OPERATOR_MENU_GAP_SMALL = 54;
Expand Down Expand Up @@ -66,13 +84,16 @@ export class OperatorsComponent implements OnInit, AfterContentInit, OnDestroy {
public categories: string[];
private _onDestroy = new Subject();

public searchInput: FormControl;

constructor(
private _breakpointObserver: BreakpointObserver,
private _operatorMenuService: OperatorMenuService,
@Inject(OPERATORS_TOKEN) public operators: OperatorDoc[]
) {}

ngOnInit() {
this.searchInput = new FormControl();
this.groupedOperators = groupOperatorsByType(this.operators);
this.categories = Object.keys(this.groupedOperators);
}
Expand Down
7 changes: 6 additions & 1 deletion src/app/operators/operators.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { WalkthroughComponent } from './components/walkthrough/walkthrough.compo
import { HighlightJsDirective } from './directives/highlight-js.directive';
import { SafeUrlPipe } from './pipes/safe-url.pipe';
import { MaterialModule } from '../material/material.module';
import { FormsModule, ReactiveFormsModule } from '@angular/forms';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the import of FormsModule isn't required. At least in the refactored component ngModel hasn't been used.

BTW, could we keep a consistent import style? Maybe the one explained in the angular style guide

import { FilterPipe } from './pipes/filter.pipe';

@NgModule({
declarations: [
Expand All @@ -35,13 +37,16 @@ import { MaterialModule } from '../material/material.module';
WalkthroughComponent,
MarbleDiagramComponent,
HighlightJsDirective,
SafeUrlPipe
SafeUrlPipe,
FilterPipe
],
imports: [
CommonModule,
MaterialModule,
OperatorsRoutingModule,
LayoutModule,
FormsModule,
ReactiveFormsModule,
TranslateModule
],
providers: [
Expand Down
13 changes: 13 additions & 0 deletions src/app/operators/pipes/filter.pipe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Pipe, PipeTransform } from '@angular/core';

@Pipe({
name: 'filter'
})
export class FilterPipe implements PipeTransform {
transform(items: any[], filter: string): any {
if (!items || !filter) {
return items;
}
return items.filter(item => item.name.indexOf(filter) !== -1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don´t like the pipe approach for this case.

Having no type is a two edged blade: doesn't tight the use of the pipe with a concrete type, but you lose type check at compile time.

}
}
3 changes: 2 additions & 1 deletion src/app/operators/specs/operators.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../operators.component';
import { OperatorDoc } from '../../../operator-docs';
import { OperatorMenuService } from '../../core/services/operator-menu.service';
import { FilterPipe } from '../pipes/filter.pipe';

const mockActivatedRoute = {
snapshot: {},
Expand Down Expand Up @@ -40,7 +41,7 @@ describe('Operators', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule, LayoutModule],
declarations: [OperatorsComponent],
declarations: [OperatorsComponent, FilterPipe],
providers: [
OperatorMenuService,
{ provide: OPERATORS_TOKEN, useValue: mockOperators },
Expand Down