Skip to content

Table multiselect filters #823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 7, 2021
4 changes: 4 additions & 0 deletions projects/common/src/utilities/lang/lang-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ const ignoreFunctions = (first: unknown, second: unknown) => {
// tslint:disable-next-line: no-null-undefined-union
export const isNonEmptyString = (str: string | undefined | null): str is string =>
str !== undefined && str !== null && str !== '';

export const hasOwnProperty = <X extends {}, Y extends PropertyKey>(obj: X, prop: Y): obj is X & Record<Y, unknown> =>
// Since Typescript doesn't know how to type guard native hasOwnProperty, we wrap it here.
obj.hasOwnProperty(prop);
95 changes: 60 additions & 35 deletions projects/components/src/table/controls/table-controls-api.ts
Original file line number Diff line number Diff line change
@@ -1,58 +1,83 @@
import { Dictionary } from '@hypertrace/common';
import { SelectOption } from '../../select/select-option';
import { FilterOperator } from '../../filtering/filter/filter-operators';
import { TableFilter } from '../table-api';

export interface SelectControl {
placeholder?: string;
default?: SelectOption<TableControlOption>;
options: SelectOption<TableControlOption>[];
export const enum TableControlOptionType {
Filter = 'filter',
Property = 'property',
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a property control option? any visual example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see this comment below.

Unset = 'unset'
}

export interface SelectChange {
select: SelectControl;
value: TableControlOption;
}
export type TableControlOption = TableUnsetControlOption | TableFilterControlOption | TablePropertyControlOption;

export interface CheckboxControl {
export interface TableFilterControlOption {
type: TableControlOptionType.Filter;
label: string;
value: boolean;
options: TableCheckboxOptions;
metaValue: TableFilter;
}

export interface CheckboxChange {
checkbox: CheckboxControl;
option: TableControlOption<boolean>;
export interface TableUnsetControlOption {
type: TableControlOptionType.Unset;
label: string;
metaValue: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is meta value supposed to do? How is it different than 'value' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see this comment below.

}

export const enum TableControlOptionType {
Filter = 'filter',
Property = 'property',
UnsetFilter = 'unset-filter'
export interface TablePropertyControlOption {
type: TableControlOptionType.Property;
label: string;
metaValue: Dictionary<unknown>;
}

export type TableControlOption<T = unknown> =
| TableUnsetFilterControlOption<T>
| TableFilterControlOption<T>
| TablePropertyControlOption<T>;
/*
* Select Control
*/

interface TableControlOptionBase<T> {
value?: T;
export interface TableSelectControl {
placeholder: string;
options: TableSelectControlOption[];
}
export interface TableUnsetFilterControlOption<T = unknown> extends TableControlOptionBase<T> {
type: TableControlOptionType.UnsetFilter;
metaValue: string;

export interface TableSelectChange {
select: TableSelectControl;
values: TableSelectControlOption[];
}

export interface TableFilterControlOption<T = unknown> extends TableControlOptionBase<T> {
type: TableControlOptionType.Filter;
metaValue: TableFilter;
export type TableSelectControlOption = TableFilterControlOption;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need so many different types? Like below where you specify metaValue type

export interface TableControlOption<T> {
  type: TableControlOptionType;
  label: string;
  metaValue: T;
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add them just for fun! 😂

But valid question. I explain a little more in this comment.


/*
* Checkbox Control
*/

export interface TableCheckboxControl {
label: string;
value: boolean;
options: TableCheckboxOptions;
}

export interface TablePropertyControlOption<T = unknown> extends TableControlOptionBase<T> {
type: TableControlOptionType.Property;
metaValue: Dictionary<unknown>;
export interface TableCheckboxChange {
checkbox: TableCheckboxControl;
option: TableCheckboxControlOption;
}

export type TableCheckboxControlOption<T extends boolean> = TableControlOption<T> & { label: string };
export type TableCheckboxControlOption<T = boolean> = TableControlOption & {
value: T;
};

export type TableCheckboxOptions = [TableCheckboxControlOption<true>, TableCheckboxControlOption<false>];

/*
* Util
*/

export const toInFilter = (tableFilters: TableFilter[]): TableFilter =>
tableFilters.reduce((previousValue, currentValue) => {
if (currentValue.operator !== FilterOperator.Equals || previousValue.field !== currentValue.field) {
throw Error('Filters must all contain same field and use = operator');
}

return {
field: previousValue.field,
operator: FilterOperator.In,
value: [...(Array.isArray(previousValue.value) ? previousValue.value : [previousValue.value]), currentValue.value]
};
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { fakeAsync } from '@angular/core/testing';
import { SubscriptionLifecycle } from '@hypertrace/common';
import { SelectComponent } from '@hypertrace/components';
import { MultiSelectComponent } from '@hypertrace/components';
import { createHostFactory, mockProvider } from '@ngneat/spectator/jest';
import { MockComponent } from 'ng-mocks';
import { SearchBoxComponent } from '../../search-box/search-box.component';
Expand All @@ -14,7 +14,7 @@ describe('Table Controls component', () => {
providers: [mockProvider(SubscriptionLifecycle)],
declarations: [
MockComponent(SearchBoxComponent),
MockComponent(SelectComponent),
MockComponent(MultiSelectComponent),
MockComponent(ToggleGroupComponent)
],
template: `
Expand Down Expand Up @@ -67,19 +67,19 @@ describe('Table Controls component', () => {
options: [
{
label: 'first1',
value: 'first-1'
metaValue: 'first-1'
},
{
label: 'second1',
value: 'second-1'
metaValue: 'second-1'
}
]
}
]
}
});

expect(spectator.query(SelectComponent)?.placeholder).toEqual('test1');
expect(spectator.query(MultiSelectComponent)?.placeholder).toEqual('test1');
});

test('should emit selection when selected', () => {
Expand All @@ -106,10 +106,12 @@ describe('Table Controls component', () => {
}
});

spectator.triggerEventHandler(SelectComponent, 'selectedChange', {
label: 'first1',
value: 'first-1'
});
spectator.triggerEventHandler(MultiSelectComponent, 'selectedChange', [
{
label: 'first1',
value: 'first-1'
}
]);

expect(onChangeSpy).toHaveBeenCalled();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@ import { Subject } from 'rxjs';
import { debounceTime } from 'rxjs/operators';
import { IconSize } from '../../icon/icon-size';
import { MultiSelectJustify } from '../../multi-select/multi-select-justify';
import { TriggerLabelDisplayMode } from '../../multi-select/multi-select.component';
import { MultiSelectSearchMode, TriggerLabelDisplayMode } from '../../multi-select/multi-select.component';
import { ToggleItem } from '../../toggle-group/toggle-item';
import { CheckboxChange, CheckboxControl, SelectChange, SelectControl, TableControlOption } from './table-controls-api';
import {
TableCheckboxChange,
TableCheckboxControl,
TableSelectChange,
TableSelectControl,
TableSelectControlOption
} from './table-controls-api';

@Component({
selector: 'ht-table-controls',
Expand All @@ -36,19 +42,20 @@ import { CheckboxChange, CheckboxControl, SelectChange, SelectControl, TableCont
></ht-search-box>

<!-- Selects -->
<ht-select
<ht-multi-select
*ngFor="let selectControl of this.selectControls"
[placeholder]="selectControl.placeholder"
[selected]="selectControl.default?.value"
class="control select"
(selectedChange)="this.onSelectChange(selectControl, $event)"
showBorder="true"
searchMode="${MultiSelectSearchMode.CaseInsensitive}"
(selectedChange)="this.onMultiSelectChange(selectControl, $event)"
>
<ht-select-option
*ngFor="let option of selectControl.options"
[label]="option.label"
[value]="option.value"
[value]="option"
></ht-select-option>
</ht-select>
</ht-multi-select>
</div>

<!-- Right -->
Expand Down Expand Up @@ -86,17 +93,18 @@ import { CheckboxChange, CheckboxControl, SelectChange, SelectControl, TableCont
})
export class TableControlsComponent implements OnChanges {
public readonly DEFAULT_SEARCH_PLACEHOLDER: string = 'Search...';

@Input()
public searchEnabled?: boolean;

@Input()
public searchPlaceholder?: string;

@Input()
public selectControls?: SelectControl[] = [];
public selectControls?: TableSelectControl[] = [];

@Input()
public checkboxControls?: CheckboxControl[] = [];
public checkboxControls?: TableCheckboxControl[] = [];

@Input()
public activeFilterItem?: ToggleItem;
Expand All @@ -111,10 +119,10 @@ export class TableControlsComponent implements OnChanges {
public readonly searchChange: EventEmitter<string> = new EventEmitter<string>();

@Output()
public readonly selectChange: EventEmitter<SelectChange> = new EventEmitter<SelectChange>();
public readonly selectChange: EventEmitter<TableSelectChange> = new EventEmitter<TableSelectChange>();

@Output()
public readonly checkboxChange: EventEmitter<CheckboxChange> = new EventEmitter<CheckboxChange>();
public readonly checkboxChange: EventEmitter<TableCheckboxChange> = new EventEmitter<TableCheckboxChange>();

@Output()
public readonly viewChange: EventEmitter<string> = new EventEmitter<string>();
Expand Down Expand Up @@ -179,21 +187,21 @@ export class TableControlsComponent implements OnChanges {
this.searchDebounceSubject.next(text);
}

public onSelectChange(select: SelectControl, value: TableControlOption): void {
public onMultiSelectChange(select: TableSelectControl, selections: TableSelectControlOption[]): void {
this.selectChange.emit({
select: select,
value: value
values: selections
});
}

public onCheckboxChange(checks: string[]): void {
const diff = this.checkboxDiffer?.diff(checks);
public onCheckboxChange(checked: string[]): void {
const diff = this.checkboxDiffer?.diff(checked);
if (!diff) {
return;
}

diff.forEachAddedItem(addedItem => {
const found: CheckboxControl | undefined = this.checkboxControls?.find(
const found: TableCheckboxControl | undefined = this.checkboxControls?.find(
control => control.label === addedItem.item
);
if (found) {
Expand All @@ -205,7 +213,7 @@ export class TableControlsComponent implements OnChanges {
});

diff.forEachRemovedItem(removedItem => {
const found: CheckboxControl | undefined = this.checkboxControls?.find(
const found: TableCheckboxControl | undefined = this.checkboxControls?.find(
control => control.label === removedItem.item
);
if (found) {
Expand All @@ -216,7 +224,7 @@ export class TableControlsComponent implements OnChanges {
}
});

this.checkboxSelections = checks;
this.checkboxSelections = checked;
this.checkboxDiffer?.diff(this.checkboxSelections);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { TableControlOption } from '@hypertrace/components';
import { Observable } from 'rxjs';
import { LabeledTableControlOption } from '../../widgets/table/table-widget-control.model';
import { GraphQlDataSourceModel } from './graphql-data-source.model';

export abstract class GraphqlTableControlOptionsDataSourceModel extends GraphQlDataSourceModel<
LabeledTableControlOption[]
> {
public abstract getData(): Observable<LabeledTableControlOption[]>;
export abstract class GraphqlTableControlOptionsDataSourceModel extends GraphQlDataSourceModel<TableControlOption[]> {
public abstract getData(): Observable<TableControlOption[]>;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { TableCheckboxOptions, TableControlOption } from '@hypertrace/components';
import { hasOwnProperty } from '@hypertrace/common';
import { TableCheckboxControlOption, TableCheckboxOptions, TableControlOption } from '@hypertrace/components';
import { BOOLEAN_PROPERTY, Model, ModelProperty } from '@hypertrace/hyperdash';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
Expand All @@ -7,7 +8,7 @@ import { TableWidgetControlModel } from './table-widget-control.model';
@Model({
type: 'table-widget-checkbox-option'
})
export class TableWidgetControlCheckboxOptionModel extends TableWidgetControlModel {
export class TableWidgetControlCheckboxOptionModel extends TableWidgetControlModel<TableCheckboxControlOption> {
@ModelProperty({
key: 'checked',
displayName: 'Checked',
Expand All @@ -19,7 +20,7 @@ export class TableWidgetControlCheckboxOptionModel extends TableWidgetControlMod
public getOptions(): Observable<TableCheckboxOptions> {
return super.getOptions().pipe(
map(options => {
if (!this.isValidCheckboxControlOption(options)) {
if (!this.isValidCheckboxControlOptions(options)) {
throw Error(`Invalid table widget checkbox data source for options '${JSON.stringify(options)}'`);
}

Expand All @@ -31,7 +32,10 @@ export class TableWidgetControlCheckboxOptionModel extends TableWidgetControlMod
);
}

private isValidCheckboxControlOption(options: TableControlOption[]): options is TableCheckboxOptions {
return options.length === 2 && options.every(option => typeof option.value === 'boolean');
private isValidCheckboxControlOptions(options: TableControlOption[]): options is TableCheckboxOptions {
return (
options.length === 2 &&
options.every(option => hasOwnProperty(option, 'value') && typeof option.value === 'boolean')
Copy link
Contributor

Choose a reason for hiding this comment

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

I am yet to understand the inheritance in the table control options. value property seems to be defined at only one place. We can also use in here, i think

Copy link
Contributor Author

@jake-bassett jake-bassett May 6, 2021

Choose a reason for hiding this comment

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

Yeah, they are confusing. Fortunately it got a little bit cleaner with this PR.

Correct, that value is only for checkbox control now, since it has to hold a true or false while the select control doesn't. Checkbox controls are actually the oddball because they have to hold more ways of modifying a query.

Both select and checkbox controls have a metaValue. This is what contains the information about what to use in the query. It is most typically a Filter (see the metaValue for TableFilterControlOption), containing key, operator, and value. This is the metaValue that all select controls use now.

There are two other cases with query info, but they are now only used for checkbox controls. In addition to the Filter metaValue, there is an Unset and a Property metaValue (see the metaValue for TableUnsetControlOption and TablePropertyControlOption). These are also used to modify the query, but in a different way.

The Unset metaValue is used to hold a string that identifies which filter to remove from our filter list. The Property metaValue is a Dictionary used to add request properties to the query. Currently, our only example of this is includeInactive: true/false that we add to the request for APIs.

So, the reason that checkbox has a value and not the select dropdown is that checkbox has this additional user specified true/false in addition to the value that is buried in the metaValue for all controls.

Not sure if that explains it or makes it more confusing, but happy to expand on anything.

);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
import { TableControlOption, TableControlOptionType, TableSelectControlOption } from '@hypertrace/components';
import { Model, ModelProperty, STRING_PROPERTY } from '@hypertrace/hyperdash';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { TableWidgetControlModel } from './table-widget-control.model';

@Model({
type: 'table-widget-select-option'
})
export class TableWidgetControlSelectOptionModel extends TableWidgetControlModel {
export class TableWidgetControlSelectOptionModel extends TableWidgetControlModel<TableSelectControlOption> {
@ModelProperty({
key: 'placeholder',
displayName: 'Placeholder',
type: STRING_PROPERTY.type
type: STRING_PROPERTY.type,
required: true
})
public placeholder?: string;
public placeholder!: string;

public getOptions(): Observable<TableSelectControlOption[]> {
return super.getOptions().pipe(
map(options => {
if (!this.isValidSelectControlOptions(options)) {
throw Error(`Invalid table widget select data source for options '${JSON.stringify(options)}'`);
}

return options;
})
);
}

private isValidSelectControlOptions(options: TableControlOption[]): options is TableSelectControlOption[] {
return options.every(option => option.type === TableControlOptionType.Filter);
}
}
Loading