From c01e0bcec3063fee89fb08f2598c55d06f0214bd Mon Sep 17 00:00:00 2001
From: hoonji <736199+hoonji@users.noreply.github.com>
Date: Tue, 26 Mar 2024 13:37:56 +0900
Subject: [PATCH] Changes custom modal to be created dynamically (#6799)
## Motivation for features / changes
The current Custom Modal can't display modals outside of their
ancestors' stacking context (e.g. in scalar card tables:
https://github.com/tensorflow/tensorboard/pull/6737#discussion_r1469588008
), which significantly hinders scalar table context menu functionality.
It also has some minor tricky issues that are difficult to fix like some
modals lingering when another one is opened:
## Technical description of changes
- Before: Custom modals were defined in a consumer component's template.
The modals were embedded views of the consumer component (e.g.
DataTableComponent), which prevented them from being displayed outside
the table's stacking context, e.g. outside of a scalar card table
- After: Custom modals are still defined in a consumer component's
template, but wrapped in ng-templates. They are dynamically created
using a newly defined CustomModal service. When the appropriate
CustomModal service method is a called, the modal template is attached
as an embedded view using CDK Overlay, which places it in a top-level overlay container, freeing it from all stacking context issues.
CustomModalComponent is completely subsumed by Overlay and is thus
deleted. Only the CustomModal service will be required going forward.
## Detailed steps to verify changes work correctly (as executed by you)
Manually tested all custom modal features in runs table, filter bar,
scalar card table
(Need to set query param `enableScalarColumnContextMenus=true` to test
scalar table context menu behavior)
---
tensorboard/webapp/angular/BUILD | 9 +
.../runs_table/filterbar_component.ng.html | 5 +-
.../views/runs_table/filterbar_component.ts | 29 +-
.../runs/views/runs_table/filterbar_test.ts | 43 +-
.../views/runs_table/runs_table_module.ts | 2 -
tensorboard/webapp/widgets/custom_modal/BUILD | 9 +-
.../widgets/custom_modal/custom_modal.ts | 170 ++++++++
.../custom_modal/custom_modal_component.ts | 133 ------
.../custom_modal/custom_modal_module.ts | 25 --
.../widgets/custom_modal/custom_modal_test.ts | 406 ++++++++++++------
.../data_table/column_selector_component.ts | 10 +-
.../data_table/data_table_component.ng.html | 20 +-
.../data_table/data_table_component.ts | 126 +++---
.../widgets/data_table/data_table_module.ts | 2 -
.../widgets/data_table/data_table_test.ts | 37 +-
15 files changed, 592 insertions(+), 434 deletions(-)
create mode 100644 tensorboard/webapp/widgets/custom_modal/custom_modal.ts
delete mode 100644 tensorboard/webapp/widgets/custom_modal/custom_modal_component.ts
delete mode 100644 tensorboard/webapp/widgets/custom_modal/custom_modal_module.ts
diff --git a/tensorboard/webapp/angular/BUILD b/tensorboard/webapp/angular/BUILD
index 5b92205186..71db97e5dc 100644
--- a/tensorboard/webapp/angular/BUILD
+++ b/tensorboard/webapp/angular/BUILD
@@ -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",
diff --git a/tensorboard/webapp/runs/views/runs_table/filterbar_component.ng.html b/tensorboard/webapp/runs/views/runs_table/filterbar_component.ng.html
index 00c6034e2c..d9a317e7e2 100644
--- a/tensorboard/webapp/runs/views/runs_table/filterbar_component.ng.html
+++ b/tensorboard/webapp/runs/views/runs_table/filterbar_component.ng.html
@@ -31,12 +31,11 @@
-
+
-
+
diff --git a/tensorboard/webapp/runs/views/runs_table/filterbar_component.ts b/tensorboard/webapp/runs/views/runs_table/filterbar_component.ts
index 29dd125abd..369cf2e838 100644
--- a/tensorboard/webapp/runs/views/runs_table/filterbar_component.ts
+++ b/tensorboard/webapp/runs/views/runs_table/filterbar_component.ts
@@ -19,6 +19,8 @@ import {
EventEmitter,
Component,
ViewChild,
+ TemplateRef,
+ ViewContainerRef,
} from '@angular/core';
import {
DiscreteFilter,
@@ -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',
@@ -41,8 +43,8 @@ export class FilterbarComponent {
@Output() removeHparamFilter = new EventEmitter();
@Output() addFilter = new EventEmitter();
- @ViewChild('filterModal', {static: false})
- private readonly filterModal!: CustomModalComponent;
+ @ViewChild('filterModalTemplate', {read: TemplateRef})
+ filterModalTemplate!: TemplateRef;
private internalSelectedFilterName = '';
get selectedFilterName(): string {
@@ -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) {
diff --git a/tensorboard/webapp/runs/views/runs_table/filterbar_test.ts b/tensorboard/webapp/runs/views/runs_table/filterbar_test.ts
index d3966cf3d2..ff6e547e2d 100644
--- a/tensorboard/webapp/runs/views/runs_table/filterbar_test.ts
+++ b/tensorboard/webapp/runs/views/runs_table/filterbar_test.ts
@@ -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';
@@ -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,
@@ -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,
@@ -61,6 +60,14 @@ const fakeFilterMap = new Map([
['filter2', intervalFilter],
]);
+@Component({
+ selector: 'testable-component',
+ template: ` `,
+})
+class TestableComponent {
+ constructor(readonly customModal: CustomModal) {}
+}
+
describe('hparam_filterbar', () => {
let actualActions: Action[];
let store: MockStore;
@@ -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();
@@ -85,23 +91,26 @@ describe('hparam_filterbar', () => {
store?.resetSelectors();
});
- function createComponent(): ComponentFixture {
+ function createComponent(): ComponentFixture {
store = TestBed.inject>(Store) as MockStore;
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 () => {
@@ -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');
});
diff --git a/tensorboard/webapp/runs/views/runs_table/runs_table_module.ts b/tensorboard/webapp/runs/views/runs_table/runs_table_module.ts
index dac151a753..247a6b1e39 100644
--- a/tensorboard/webapp/runs/views/runs_table/runs_table_module.ts
+++ b/tensorboard/webapp/runs/views/runs_table/runs_table_module.ts
@@ -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';
@@ -68,7 +67,6 @@ import {RunsTableContainer} from './runs_table_container';
MatSortModule,
MatTableModule,
RangeInputModule,
- CustomModalModule,
AlertModule,
],
exports: [RunsTableContainer],
diff --git a/tensorboard/webapp/widgets/custom_modal/BUILD b/tensorboard/webapp/widgets/custom_modal/BUILD
index d600bb74ef..7da319b9fa 100644
--- a/tensorboard/webapp/widgets/custom_modal/BUILD
+++ b/tensorboard/webapp/widgets/custom_modal/BUILD
@@ -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",
],
@@ -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",
],
)
diff --git a/tensorboard/webapp/widgets/custom_modal/custom_modal.ts b/tensorboard/webapp/widgets/custom_modal/custom_modal.ts
new file mode 100644
index 0000000000..90d3b689c7
--- /dev/null
+++ b/tensorboard/webapp/widgets/custom_modal/custom_modal.ts
@@ -0,0 +1,170 @@
+/* Copyright 2024 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
+import {Injectable, TemplateRef, ViewContainerRef} from '@angular/core';
+import {ConnectedPosition, Overlay, OverlayRef} from '@angular/cdk/overlay';
+import {TemplatePortal} from '@angular/cdk/portal';
+import {Subject, Subscription} from 'rxjs';
+import {isMouseEventInElement} from '../../util/dom';
+
+/**
+ * Enables dynamic creation of modal components.
+ *
+ * # Usage
+ * Define a modal using an ng-template:
+ * ```
+ *
+ *
+ *
+ *
+ * ```
+ *
+ * Define a ViewChild to reference the template in the component file:
+ * ```
+ * // my_component.ts
+ * ...
+ * @ViewChild('myModalTemplate', {read: TemplateRef})
+ * myModalTemplate!: TemplateRef;
+ * ...
+ * ```
+ *
+ * Inject CustomModal and ViewContainerRef into the component
+ * ```
+ * // my_component.ts
+ * ...
+ * constructor(
+ * private readonly customModal: CustomModal,
+ * private readonly viewContainerRef: ViewContainerRef,
+ * ) {}
+ * ...
+ * ```
+ *
+ * To create a modal, call createNextToElement():
+ * ```
+ * // my_component.ts
+ * ...
+ * onSomeButtonClick(mouseEvent: MouseEventj) {
+ * this.customModal.createNextToElement(
+ * this.myModalTemplate,
+ * mouseEvent.target as HTMLElement,
+ * this.viewContainerRef
+ * );
+ * }
+ * ...
+ * ```
+ */
+
+export class CustomModalRef {
+ overlayRef: OverlayRef;
+ subscriptions: Subscription[] = [];
+ onClose = new Subject();
+
+ constructor(overlayRef: OverlayRef) {
+ this.overlayRef = overlayRef;
+ }
+}
+
+@Injectable({providedIn: 'root'})
+export class CustomModal {
+ private customModalRefs: CustomModalRef[] = [];
+
+ constructor(private readonly overlay: Overlay) {}
+
+ /** Creates a modal from a template next to an element. */
+ createNextToElement(
+ templateRef: TemplateRef,
+ element: Element,
+ viewContainerRef: ViewContainerRef,
+ connectedPosition: ConnectedPosition = {
+ originX: 'end',
+ originY: 'top',
+ overlayX: 'start',
+ overlayY: 'top',
+ }
+ ): CustomModalRef | undefined {
+ let positionStrategy = this.overlay.position().flexibleConnectedTo(element);
+ if (connectedPosition) {
+ positionStrategy = positionStrategy.withPositions([connectedPosition]);
+ }
+
+ const overlayRef = this.overlay.create({
+ positionStrategy,
+ hasBackdrop: false,
+ });
+ overlayRef.attach(new TemplatePortal(templateRef, viewContainerRef));
+ const customModalRef = new CustomModalRef(overlayRef);
+ this.customModalRefs.push(customModalRef);
+
+ // setTimeout to prevent closing immediately after modal open.
+ setTimeout(() => {
+ const outsidePointerEventsSubscription = overlayRef
+ .outsidePointerEvents()
+ .subscribe((event) => {
+ // Only close when click is outside of every modal
+ if (
+ this.customModalRefs.every(
+ (ref) =>
+ !isMouseEventInElement(event, ref.overlayRef.overlayElement)
+ )
+ ) {
+ this.closeAll();
+ }
+ });
+ customModalRef.subscriptions.push(outsidePointerEventsSubscription);
+ });
+
+ const keydownEventsSubscription = overlayRef
+ .keydownEvents()
+ .subscribe((event) => {
+ if (event.key === 'Escape') {
+ this.closeAll();
+ }
+ });
+ customModalRef.subscriptions.push(keydownEventsSubscription);
+
+ return customModalRef;
+ }
+
+ /** Destroys given custom modal and related resources. */
+ close(customModalRef: CustomModalRef) {
+ const index = this.customModalRefs.findIndex(
+ (ref) => ref === customModalRef
+ );
+ if (index === -1) {
+ console.warn('Could not find customModalRef', customModalRef);
+ return;
+ }
+
+ customModalRef.subscriptions.forEach((subscription) => {
+ subscription.unsubscribe();
+ });
+ customModalRef.subscriptions = [];
+ customModalRef.overlayRef?.dispose();
+
+ this.customModalRefs.splice(index, 1);
+
+ customModalRef.onClose.next();
+ customModalRef.onClose.complete();
+ }
+
+ /** Destroys all created modals. */
+ closeAll() {
+ while (this.customModalRefs.length) {
+ this.close(this.customModalRefs[0]);
+ }
+ }
+}
diff --git a/tensorboard/webapp/widgets/custom_modal/custom_modal_component.ts b/tensorboard/webapp/widgets/custom_modal/custom_modal_component.ts
deleted file mode 100644
index a2e1a22539..0000000000
--- a/tensorboard/webapp/widgets/custom_modal/custom_modal_component.ts
+++ /dev/null
@@ -1,133 +0,0 @@
-/* Copyright 2023 The TensorFlow Authors. All Rights Reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-==============================================================================*/
-import {
- Component,
- EventEmitter,
- Output,
- ViewChild,
- ElementRef,
- HostListener,
- OnInit,
- ViewContainerRef,
-} from '@angular/core';
-import {BehaviorSubject} from 'rxjs';
-
-export interface ModalContent {
- onRender?: () => void;
-}
-
-@Component({
- selector: 'custom-modal',
- template: `
-
-
-
-
-
- `,
- styles: [
- `
- :host {
- position: fixed;
- left: 0;
- z-index: 9001;
- }
-
- .content {
- position: absolute;
- }
- `,
- ],
-})
-export class CustomModalComponent implements OnInit {
- @Output() onOpen = new EventEmitter();
- @Output() onClose = new EventEmitter();
-
- readonly visible$ = new BehaviorSubject(false);
- private canClose = true;
-
- @ViewChild('content', {static: false})
- private readonly content!: ElementRef;
-
- private clickListener: () => void = this.maybeClose.bind(this);
-
- constructor(private readonly viewRef: ViewContainerRef) {}
-
- ngOnInit() {
- this.visible$.subscribe((visible) => {
- // Wait until after the next browser frame.
- window.requestAnimationFrame(() => {
- this.canClose = true;
- if (visible) {
- this.ensureContentIsWithinWindow();
- this.onOpen.emit();
- } else {
- this.onClose.emit();
- }
- });
- });
- }
-
- public openAtPosition(position: {x: number; y: number}) {
- const root = this.viewRef.element.nativeElement;
- // Set left/top to viewport (0,0) if the element has another "containing block" ancestor.
- root.style.top = `${root.offsetTop - root.getBoundingClientRect().top}px`;
- root.style.left = `${
- root.offsetLeft - root.getBoundingClientRect().left
- }px`;
-
- this.content.nativeElement.style.left = position.x + 'px';
- this.content.nativeElement.style.top = position.y + 'px';
- this.canClose = false;
- this.visible$.next(true);
- document.addEventListener('click', this.clickListener);
- }
-
- private ensureContentIsWithinWindow() {
- if (!this.content) {
- return;
- }
-
- const boundingBox = this.content.nativeElement.getBoundingClientRect();
- if (boundingBox.left < 0) {
- this.content.nativeElement.style.left = 0;
- }
- if (boundingBox.left + boundingBox.width > window.innerWidth) {
- this.content.nativeElement.style.left =
- window.innerWidth - boundingBox.width + 'px';
- }
-
- if (boundingBox.top < 0) {
- this.content.nativeElement.style.top = 0;
- }
- if (boundingBox.top + boundingBox.height > window.innerHeight) {
- this.content.nativeElement.style.top =
- window.innerHeight - boundingBox.height + 'px';
- }
- }
-
- @HostListener('document:keydown.escape', ['$event'])
- private maybeClose() {
- if (!this.canClose) {
- return;
- }
- this.close();
- }
-
- public close() {
- document.removeEventListener('click', this.clickListener);
- this.visible$.next(false);
- }
-}
diff --git a/tensorboard/webapp/widgets/custom_modal/custom_modal_module.ts b/tensorboard/webapp/widgets/custom_modal/custom_modal_module.ts
deleted file mode 100644
index 85f509ed2e..0000000000
--- a/tensorboard/webapp/widgets/custom_modal/custom_modal_module.ts
+++ /dev/null
@@ -1,25 +0,0 @@
-/* Copyright 2023 The TensorFlow Authors. All Rights Reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-==============================================================================*/
-
-import {CommonModule} from '@angular/common';
-import {NgModule} from '@angular/core';
-import {CustomModalComponent} from './custom_modal_component';
-
-@NgModule({
- declarations: [CustomModalComponent],
- imports: [CommonModule],
- exports: [CustomModalComponent],
-})
-export class CustomModalModule {}
diff --git a/tensorboard/webapp/widgets/custom_modal/custom_modal_test.ts b/tensorboard/webapp/widgets/custom_modal/custom_modal_test.ts
index 406290ee99..3a945450fb 100644
--- a/tensorboard/webapp/widgets/custom_modal/custom_modal_test.ts
+++ b/tensorboard/webapp/widgets/custom_modal/custom_modal_test.ts
@@ -12,183 +12,309 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
-import {ComponentFixture, TestBed} from '@angular/core/testing';
+import {
+ ComponentFixture,
+ TestBed,
+ fakeAsync,
+ tick,
+} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
-import {CustomModalComponent} from './custom_modal_component';
+import {CustomModal} from './custom_modal';
import {CommonModule} from '@angular/common';
import {
Component,
- ElementRef,
- EventEmitter,
- Output,
+ TemplateRef,
ViewChild,
+ ViewContainerRef,
} from '@angular/core';
-
-function waitFrame() {
- return new Promise((resolve) => window.requestAnimationFrame(resolve));
-}
+import {Overlay, OverlayModule, OverlayRef} from '@angular/cdk/overlay';
+import {first} from 'rxjs';
@Component({
- selector: 'testable-modal',
- template: `
- My great content
- `,
-})
-class TestableComponent {
- @ViewChild('modal', {static: false})
- modalComponent!: CustomModalComponent;
-
- @ViewChild('content', {static: false})
- content!: ElementRef;
-
- isOpen = false;
-
- @Output() onOpen = new EventEmitter();
- @Output() onClose = new EventEmitter();
-
- setOpen() {
- this.isOpen = true;
- this.onOpen.emit();
- }
+ selector: 'fake-modal-view-container',
+ template: `
+
+
+
+ abc123
+
+
+ xyz
+
+ `,
+ styles: [
+ `
+ :host {
+ display: block;
+ width: 400px;
+ height: 400px;
+ }
- setClosed() {
- this.isOpen = false;
- this.onClose.emit();
- }
-
- close() {
- this.modalComponent.close();
- }
+ .content,
+ .another-content {
+ // Make modals small to allow easily testing clicking outside of modals.
+ width: 10px;
+ height: 10px;
+ }
+ `,
+ ],
+})
+class FakeViewContainerComponent {
+ @ViewChild('modalTemplate', {read: TemplateRef})
+ readonly modalTemplateRef!: TemplateRef;
- getContentStyle() {
- return (this.modalComponent as any).content.nativeElement.style;
- }
+ @ViewChild('anotherModalTemplate', {read: TemplateRef})
+ readonly anotherModalTemplateRef!: TemplateRef;
- public openAtPosition(position: {x: number; y: number}) {
- this.modalComponent.openAtPosition(position);
- }
+ constructor(
+ readonly customModal: CustomModal,
+ readonly vcRef: ViewContainerRef
+ ) {}
}
describe('custom modal', () => {
+ let viewContainerFixture: ComponentFixture;
+
beforeEach(async () => {
await TestBed.configureTestingModule({
- declarations: [TestableComponent, CustomModalComponent],
- imports: [CommonModule],
+ declarations: [FakeViewContainerComponent],
+ imports: [CommonModule, OverlayModule],
}).compileComponents();
+
+ viewContainerFixture = TestBed.createComponent(FakeViewContainerComponent);
+ viewContainerFixture.detectChanges();
});
- it('waits a frame before emitting onOpen or onClose', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 0, y: 0});
- expect(fixture.componentInstance.isOpen).toBeFalse();
- await waitFrame();
- expect(fixture.componentInstance.isOpen).toBeTrue();
- fixture.componentInstance.close();
- fixture.detectChanges();
- await waitFrame();
- expect(fixture.componentInstance.isOpen).toBeFalse();
+ it('creates a modal', () => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const overlay = TestBed.inject(Overlay);
+ const createSpy = spyOn(overlay, 'create').and.callThrough();
+ const attachSpy = spyOn(OverlayRef.prototype, 'attach').and.callThrough();
+
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerFixture.detectChanges();
+
+ const content = viewContainerFixture.debugElement.query(By.css('.content'));
+ expect(content.nativeElement.innerHTML).toContain('abc123');
+ const createArg = createSpy.calls.mostRecent().args[0]!;
+ expect(createArg.positionStrategy).toEqual(
+ jasmine.objectContaining({
+ positions: [
+ {
+ originX: 'end',
+ originY: 'top',
+ overlayX: 'start',
+ overlayY: 'top',
+ },
+ ],
+ })
+ );
+ const attachArgs = attachSpy.calls.mostRecent().args[0];
+ expect(attachArgs.templateRef).toBe(
+ viewContainerComponent.modalTemplateRef
+ );
+ expect(attachArgs.viewContainerRef).toBe(viewContainerComponent.vcRef);
});
- describe('openAtPosition', () => {
- it('applies top and left offsets', () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 20, y: 10});
- expect(fixture.componentInstance.getContentStyle().top).toEqual('10px');
- expect(fixture.componentInstance.getContentStyle().left).toEqual('20px');
- });
+ describe('overlay event subscriptions', () => {
+ it('subscribes to click and pointer events on create', fakeAsync(() => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+
+ const customModalRef =
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ )!;
+ tick();
- it('emits onOpen', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- const spy = spyOn(fixture.componentInstance.onOpen, 'emit');
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 20, y: 10});
- expect(spy).not.toHaveBeenCalled();
- await waitFrame();
- expect(spy).toHaveBeenCalled();
+ expect(customModalRef.subscriptions.length).toEqual(2);
+ }));
+
+ it('cleans up subscriptions on removal', fakeAsync(() => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const customModalRef =
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ )!;
+ tick();
+
+ viewContainerComponent.customModal.close(customModalRef);
+
+ expect(customModalRef.subscriptions.length).toEqual(0);
+ }));
+ });
+
+ describe('closeAll', () => {
+ it('clears all modals in the modal ViewContainerRef', () => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const anotherModalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.another-modal-trigger-button')
+ ).nativeElement;
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.anotherModalTemplateRef,
+ anotherModalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerFixture.detectChanges();
+
+ TestBed.inject(CustomModal).closeAll();
+
+ const content = viewContainerFixture.debugElement.query(
+ By.css('.content')
+ );
+ const anotherContent = viewContainerFixture.debugElement.query(
+ By.css('.another-content')
+ );
+ expect(content).toBeNull();
+ expect(anotherContent).toBeNull();
+ expect(viewContainerComponent.vcRef.length).toBe(0);
});
});
describe('closing behavior', () => {
- let fixture: ComponentFixture;
- beforeEach(async () => {
- fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 0, y: 0});
- await waitFrame();
- });
+ it('emits onClose event on close', fakeAsync(() => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const customModalRef =
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ )!;
- it('closes when escape key is pressed', async () => {
- expect(fixture.componentInstance.isOpen).toBeTrue();
- const event = new KeyboardEvent('keydown', {key: 'escape'});
- document.dispatchEvent(event);
- await waitFrame();
+ customModalRef.onClose.pipe(first()).subscribe((val) => {
+ // onClose should emit an empty value.
+ expect(val).toBeUndefined();
+ });
- expect(fixture.componentInstance.isOpen).toBeFalse();
- });
+ viewContainerComponent.customModal.close(customModalRef);
+ tick();
+ }));
- it('closes when user clicks outside modal', async () => {
- expect(fixture.componentInstance.isOpen).toBeTrue();
- document.body.click();
- await waitFrame();
+ it('closes when escape key is pressed', fakeAsync(() => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerFixture.detectChanges();
+ tick();
+ const content = viewContainerFixture.debugElement.query(
+ By.css('.content')
+ );
- expect(fixture.componentInstance.isOpen).toBeFalse();
- });
- });
+ const event = new KeyboardEvent('keydown', {key: 'Escape'});
+ document.body.dispatchEvent(event);
+ viewContainerFixture.detectChanges();
+ tick();
- describe('ensures content is always within the window', () => {
- beforeEach(() => {
- window.innerHeight = 1000;
- window.innerWidth = 1000;
- });
+ expect(viewContainerComponent.vcRef.length).toBe(0);
+ expect(
+ viewContainerFixture.debugElement.query(By.css('.content'))
+ ).toBeNull();
+ }));
- it('sets left to 0 if less than 0', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: -10, y: 0});
- expect(fixture.componentInstance.isOpen).toBeFalse();
- await waitFrame();
- fixture.detectChanges();
+ it('closes all modals when user clicks an area outside all modals', fakeAsync(() => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const anotherModalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.another-modal-trigger-button')
+ ).nativeElement;
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.anotherModalTemplateRef,
+ anotherModalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerFixture.detectChanges();
+ tick();
- const content = fixture.debugElement.query(By.css('.content'));
- expect(content.nativeElement.style.left).toEqual('0px');
- });
+ const event = new MouseEvent('click', {clientX: 300, clientY: 300});
+ viewContainerFixture.nativeElement.dispatchEvent(event);
+ viewContainerFixture.detectChanges();
- it('sets top to 0 if less than 0', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 0, y: -10});
- expect(fixture.componentInstance.isOpen).toBeFalse();
- await waitFrame();
- fixture.detectChanges();
+ const content = viewContainerFixture.debugElement.query(
+ By.css('.content')
+ );
+ const anotherContent = viewContainerFixture.debugElement.query(
+ By.css('.another-content')
+ );
+ expect(content).toBeNull();
+ expect(anotherContent).toBeNull();
+ expect(viewContainerComponent.vcRef.length).toBe(0);
+ }));
- const content = fixture.debugElement.query(By.css('.content'));
- expect(content.nativeElement.style.top).toEqual('0px');
- });
+ it('does not close when a click is inside at least one modal', async () => {
+ const viewContainerComponent = viewContainerFixture.componentInstance;
+ const modalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.modal-trigger-button')
+ ).nativeElement;
+ const anotherModalTriggerButton = viewContainerFixture.debugElement.query(
+ By.css('.another-modal-trigger-button')
+ ).nativeElement;
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.modalTemplateRef,
+ modalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerComponent.customModal.createNextToElement(
+ viewContainerComponent.anotherModalTemplateRef,
+ anotherModalTriggerButton,
+ viewContainerComponent.vcRef
+ );
+ viewContainerFixture.detectChanges();
+ const content = viewContainerFixture.debugElement.query(
+ By.css('.content')
+ );
+ const anotherContent = viewContainerFixture.debugElement.query(
+ By.css('.another-content')
+ );
- it('sets left to maximum if content overflows the window', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 1010, y: 0});
- expect(fixture.componentInstance.isOpen).toBeFalse();
- await waitFrame();
- fixture.detectChanges();
- const content = fixture.debugElement.query(By.css('.content'));
- // While rendering in a test the elements width and height will appear to be 0.
- expect(content.nativeElement.style.left).toEqual('1000px');
- });
+ // Event is in first modal.
+ const event = new MouseEvent('click', {clientX: 101, clientY: 101});
+ content.nativeElement.dispatchEvent(event);
+ viewContainerFixture.detectChanges();
- it('sets top to maximum if content overflows the window', async () => {
- const fixture = TestBed.createComponent(TestableComponent);
- fixture.detectChanges();
- fixture.componentInstance.openAtPosition({x: 0, y: 1010});
- expect(fixture.componentInstance.isOpen).toBeFalse();
- await waitFrame();
- fixture.detectChanges();
- const content = fixture.debugElement.query(By.css('.content'));
- // While rendering in a test the elements width and height will appear to be 0.
- expect(content.nativeElement.style.top).toEqual('1000px');
+ expect(content.nativeElement.innerHTML).toContain('abc123');
+ expect(anotherContent.nativeElement.innerHTML).toContain('xyz');
});
});
});
diff --git a/tensorboard/webapp/widgets/data_table/column_selector_component.ts b/tensorboard/webapp/widgets/data_table/column_selector_component.ts
index 1b55a25e63..8ff421384b 100644
--- a/tensorboard/webapp/widgets/data_table/column_selector_component.ts
+++ b/tensorboard/webapp/widgets/data_table/column_selector_component.ts
@@ -93,12 +93,12 @@ export class ColumnSelectorComponent implements OnInit, AfterViewInit {
ngAfterViewInit() {
this.searchInput = '';
- this.searchField.nativeElement.focus();
this.selectedIndex$.next(0);
- }
-
- focus() {
- this.searchField?.nativeElement.focus();
+ this.activate();
+ // Wait until next tick to prevent https://angular.io/errors/NG0100
+ setTimeout(() => {
+ this.searchField?.nativeElement.focus();
+ });
}
getFilteredColumns() {
diff --git a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html
index 7c4a1145bd..656572537e 100644
--- a/tensorboard/webapp/widgets/data_table/data_table_component.ng.html
+++ b/tensorboard/webapp/widgets/data_table/data_table_component.ng.html
@@ -12,7 +12,7 @@
limitations under the License.
-->
-
+
-
+
-
+
-
+
-
+
-
+