Skip to content

Changes custom modal to be created dynamically #6799

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 12 commits into from
Mar 26, 2024
9 changes: 9 additions & 0 deletions tensorboard/webapp/angular/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,15 @@ tf_ts_library(
],
)

# This is a dummy rule used as a @angular/cdk/portal dependency.
tf_ts_library(
name = "expect_angular_cdk_portal",
srcs = [],
deps = [
"@npm//@angular/cdk",
],
)

# This is a dummy rule used as a @angular/cdk/scrolling dependency.
tf_ts_library(
name = "expect_angular_cdk_scrolling",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@
</mat-chip-set>
</div>

<custom-modal #filterModal (onClose)="deselectFilter()">
<ng-template #filterModalTemplate>
<tb-data-table-filter
*ngIf="selectedFilter"
[filter]="selectedFilter"
(intervalFilterChanged)="emitIntervalFilterChanged($event)"
(discreteFilterChanged)="emitDiscreteFilterChanged($event)"
(includeUndefinedToggled)="emitIncludeUndefinedToggled()"
></tb-data-table-filter>
</custom-modal>
</ng-template>
29 changes: 15 additions & 14 deletions tensorboard/webapp/runs/views/runs_table/filterbar_component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
EventEmitter,
Component,
ViewChild,
TemplateRef,
ViewContainerRef,
} from '@angular/core';
import {
DiscreteFilter,
Expand All @@ -27,7 +29,7 @@ import {
FilterAddedEvent,
} from '../../../widgets/data_table/types';
import {RangeValues} from '../../../widgets/range_input/types';
import {CustomModalComponent} from '../../../widgets/custom_modal/custom_modal_component';
import {CustomModal} from '../../../widgets/custom_modal/custom_modal';

@Component({
selector: 'filterbar-component',
Expand All @@ -41,8 +43,8 @@ export class FilterbarComponent {
@Output() removeHparamFilter = new EventEmitter<string>();
@Output() addFilter = new EventEmitter<FilterAddedEvent>();

@ViewChild('filterModal', {static: false})
private readonly filterModal!: CustomModalComponent;
@ViewChild('filterModalTemplate', {read: TemplateRef})
filterModalTemplate!: TemplateRef<unknown>;

private internalSelectedFilterName = '';
get selectedFilterName(): string {
Expand All @@ -56,19 +58,18 @@ export class FilterbarComponent {
return this.filters.get(this.selectedFilterName);
}

constructor(
private readonly customModal: CustomModal,
private readonly viewContainerRef: ViewContainerRef
) {}

openFilterMenu(event: MouseEvent, filterName: string) {
this.selectedFilterName = filterName;
const rect = (
(event.target as HTMLElement).closest('mat-chip') as HTMLElement
).getBoundingClientRect();
this.filterModal.openAtPosition({
x: rect.x + rect.width,
y: rect.y,
});
}

deselectFilter() {
this.selectedFilterName = '';
this.customModal.createNextToElement(
this.filterModalTemplate,
(event.target as HTMLElement).closest('mat-chip') as HTMLElement,
this.viewContainerRef
);
}

emitIntervalFilterChanged(value: RangeValues) {
Expand Down
43 changes: 26 additions & 17 deletions tensorboard/webapp/runs/views/runs_table/filterbar_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {NO_ERRORS_SCHEMA} from '@angular/core';
import {Component, NO_ERRORS_SCHEMA} from '@angular/core';
import {FilterbarComponent} from './filterbar_component';
import {FilterbarContainer} from './filterbar_container';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
Expand All @@ -22,7 +22,6 @@ import {MockStore} from '@ngrx/store/testing';
import {State} from '../../../app_state';
import {Action, Store} from '@ngrx/store';
import {By} from '@angular/platform-browser';
import {CustomModalModule} from '../../../widgets/custom_modal/custom_modal_module';
import {
actions as hparamsActions,
selectors as hparamsSelectors,
Expand All @@ -36,9 +35,9 @@ import {MatChipHarness} from '@angular/material/chips/testing';
import {TestbedHarnessEnvironment} from '@angular/cdk/testing/testbed';
import {MatChipRemove, MatChipsModule} from '@angular/material/chips';
import {MatIconTestingModule} from '../../../testing/mat_icon_module';
import {CustomModalComponent} from '../../../widgets/custom_modal/custom_modal_component';
import {FilterDialogModule} from '../../../widgets/data_table/filter_dialog_module';
import {FilterDialog} from '../../../widgets/data_table/filter_dialog_component';
import {CustomModal} from '../../../widgets/custom_modal/custom_modal';

const discreteFilter: DiscreteFilter = {
type: DomainType.DISCRETE,
Expand All @@ -61,6 +60,14 @@ const fakeFilterMap = new Map<string, DiscreteFilter | IntervalFilter>([
['filter2', intervalFilter],
]);

@Component({
selector: 'testable-component',
template: ` <filterbar></filterbar> `,
})
class TestableComponent {
constructor(readonly customModal: CustomModal) {}
}

describe('hparam_filterbar', () => {
let actualActions: Action[];
let store: MockStore<State>;
Expand All @@ -69,13 +76,12 @@ describe('hparam_filterbar', () => {
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [
CustomModalModule,
NoopAnimationsModule,
MatChipsModule,
MatIconTestingModule,
FilterDialogModule,
],
declarations: [FilterbarComponent, FilterbarContainer],
declarations: [FilterbarComponent, FilterbarContainer, TestableComponent],
providers: [provideMockTbStore()],
schemas: [NO_ERRORS_SCHEMA],
}).compileComponents();
Expand All @@ -85,23 +91,26 @@ describe('hparam_filterbar', () => {
store?.resetSelectors();
});

function createComponent(): ComponentFixture<FilterbarContainer> {
function createComponent(): ComponentFixture<TestableComponent> {
store = TestBed.inject<Store<State>>(Store) as MockStore<State>;
actualActions = [];
dispatchSpy = spyOn(store, 'dispatch').and.callFake((action: Action) => {
actualActions.push(action);
});

return TestBed.createComponent(FilterbarContainer);
const fixture = TestBed.createComponent(TestableComponent);
return fixture;
}

it('renders hparam filterbar', () => {
const fixture = createComponent();
fixture.detectChanges();

const dialog = fixture.debugElement.query(By.directive(FilterbarComponent));
const filterBarComponent = fixture.debugElement.query(
By.directive(FilterbarComponent)
);

expect(dialog).toBeTruthy();
expect(filterBarComponent).toBeTruthy();
});

it("doesn't render if no filters are set", async () => {
Expand Down Expand Up @@ -164,23 +173,23 @@ describe('hparam_filterbar', () => {
const component = fixture.debugElement.query(
By.directive(FilterbarComponent)
).componentInstance;
const openAtPositionSpy = spyOn(
CustomModalComponent.prototype,
'openAtPosition'
const createNextToElementSpy = spyOn(
TestBed.inject(CustomModal),
'createNextToElement'
);
const loader = TestbedHarnessEnvironment.loader(fixture);
fixture.detectChanges();

const chipHarness = await loader.getHarness(MatChipHarness);
const chip = await chipHarness.host();
const chipDimensions = await chip.getDimensions();
await chip.click();
fixture.detectChanges();

expect(openAtPositionSpy).toHaveBeenCalledWith({
x: chipDimensions.left + chipDimensions.width,
y: chipDimensions.top,
});
expect(createNextToElementSpy).toHaveBeenCalledWith(
component.filterModalTemplate,
fixture.debugElement.query(By.css('mat-chip')).nativeElement,
component.viewContainerRef
);
expect(component.selectedFilterName).toBe('filter1');
});

Expand Down
2 changes: 0 additions & 2 deletions tensorboard/webapp/runs/views/runs_table/runs_table_module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {FilterbarComponent} from './filterbar_component';
import {FilterbarContainer} from './filterbar_container';
import {RunsGroupMenuButtonComponent} from './runs_group_menu_button_component';
import {RunsGroupMenuButtonContainer} from './runs_group_menu_button_container';
import {CustomModalModule} from '../../../widgets/custom_modal/custom_modal_module';
import {RunsDataTable} from './runs_data_table';
import {RunsTableContainer} from './runs_table_container';

Expand All @@ -68,7 +67,6 @@ import {RunsTableContainer} from './runs_table_container';
MatSortModule,
MatTableModule,
RangeInputModule,
CustomModalModule,
AlertModule,
],
exports: [RunsTableContainer],
Expand Down
9 changes: 6 additions & 3 deletions tensorboard/webapp/widgets/custom_modal/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ package(default_visibility = ["//tensorboard:internal"])
tf_ng_module(
name = "custom_modal",
srcs = [
"custom_modal_component.ts",
"custom_modal_module.ts",
"custom_modal.ts",
],
deps = [
"@npm//@angular/common",
"//tensorboard/webapp/angular:expect_angular_cdk_overlay",
"//tensorboard/webapp/angular:expect_angular_cdk_portal",
"//tensorboard/webapp/util:dom",
"@npm//@angular/core",
"@npm//rxjs",
],
Expand All @@ -23,10 +24,12 @@ tf_ts_library(
],
deps = [
":custom_modal",
"//tensorboard/webapp/angular:expect_angular_cdk_overlay",
"//tensorboard/webapp/angular:expect_angular_core_testing",
"@npm//@angular/common",
"@npm//@angular/core",
"@npm//@angular/platform-browser",
"@npm//@types/jasmine",
"@npm//rxjs",
],
)
Loading
Loading