Changes custom modal to be created dynamically #6799
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| /* 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 { | ||
| ApplicationRef, | ||
| Injectable, | ||
| TemplateRef, | ||
| ViewContainerRef, | ||
| } from '@angular/core'; | ||
| import {CustomModalComponent} from './custom_modal_component'; | ||
|
|
||
| /** | ||
|
|
||
| * Enables dynamic creation of modal components. | ||
| * | ||
| * # Prerequisites | ||
| * App root component must define a ViewContainerRef named `modalViewContainerRef` | ||
| * e.g.: | ||
| * | ||
| * ``` | ||
| * // Template file | ||
| * <div #modal_container></div> | ||
| * ``` | ||
| * | ||
| * ``` | ||
| * // Root Component definition | ||
| * class MyAppRoot { | ||
| * @ViewChild('modal_container', {read: ViewContainerRef}) | ||
| * readonly modalViewContainerRef!: ViewContainerRef; | ||
| * ... | ||
| * } | ||
| * ``` | ||
| * | ||
| * # Usage | ||
| * Define a modal using an ng-template: | ||
| * ``` | ||
| * <ng-template #myModalTemplate> | ||
| * <custom-modal (onOpen)="doSomething" (onClose)="doSomethingElse"> | ||
| * <my-awesome-modal-content | ||
| * [input1]="input1" | ||
| * (output1)="output1" | ||
| * > | ||
| * </my-awesome-modal-content> | ||
| * </custom-modal> | ||
| * </ng-template> | ||
| * ``` | ||
| * | ||
| * Define a ViewChild to reference the template in the component file: | ||
| * ``` | ||
| * // my_component.ts | ||
| * ... | ||
| * @ViewChild('myModalTemplate', {read: TemplateRef}) | ||
| * myModalTemplate!: TemplateRef<unknown>; | ||
| * ... | ||
| * ``` | ||
| * | ||
| * Inject CustomModal into the component | ||
| * ``` | ||
| * // my_component.ts | ||
| * ... | ||
| * constructor(private readonly customModal: CustomModal) {} | ||
| * ... | ||
| * ``` | ||
| * | ||
| * To create a modal, call createAtPosition(): | ||
| * ``` | ||
| * // my_component.ts | ||
| * ... | ||
| * onSomeButtonClick() { | ||
| * this.customModal.createAtPosition(this.myModalTemplate, {x: 100, y: 100}); | ||
| * } | ||
| * ... | ||
| * ``` | ||
| * | ||
| * ## Important note | ||
| * runChangeDetection() must be called after view checked to prevent annoying | ||
| * https://angular.io/errors/NG0100 errors when using input bindings: this | ||
| * will run embedded view change detection in the correct order. Note that | ||
| * omitting this will not affect actual modal behavior. | ||
| * ``` | ||
| * // my_component.ts | ||
| * ngAfterViewChecked() { | ||
| * this.customModal.runChangeDetection(); | ||
| * } | ||
| * ``` | ||
| * | ||
| * # Testing | ||
| * A similar setup is required for testing modal behavior from another component. | ||
| * | ||
| * Define a separate root component to hold the modal container and ref: | ||
| * ``` | ||
| * @Component({ | ||
| * ..., | ||
| * template: ` | ||
| * <actual-component-to-test></actual-component-to-test> | ||
| * <div #modal_container></div> | ||
| * ` | ||
| * }) | ||
| * class TestableComponent { | ||
| * @ViewChild('modal_container', {read: ViewContainerRef}) | ||
| * readonly modalViewContainerRef!: ViewContainerRef; | ||
| * | ||
| * constructor(readonly customModal: CustomModal) {} | ||
| * ... | ||
| * } | ||
| * ``` | ||
| * | ||
| * Before each test (e.g. in beforeEach), make sure to add the TestableComponent | ||
| * to the app component list: | ||
| * ``` | ||
| * ... | ||
| * const fixture = TestBed.createComponent(TestableComponent); | ||
| * const appRef = TestBed.inject(ApplicationRef); | ||
| * appRef.components.push(fixture.componentRef); | ||
| * ``` | ||
| */ | ||
| @Injectable({providedIn: 'root'}) | ||
| export class CustomModal { | ||
|
There was a problem hiding this comment. Looks good. The problems I was having with the column selector in data table seem to have been solved! I was wondering, though, could we have instead used Angular Material Overlay? https://material.angular.io/cdk/overlay/overview There was a problem hiding this comment. Oh wow, didn't know about this! Yes I'd say the Overlay problem scope is basically equivalent to CustomModal's. I immediately tried this approach out, as it's clearly more preferable to use a mature, featureful library if possible. Unfortunately, the dynamic nature of our clickable affordances doesn't seem to be compatible with how Overlay works. To elaborate: Overlay usage (ex) seems to require referring to the clickable affordance by template variable (e.g. There was a problem hiding this comment. Overlay also contains a programmatic interface. Does that make it usable? Here I migrated the filterbar_component to use overlay: I recognize that filterbar is the easiest of the usages of CustomModal but hopefully this points you in a useful direction. I admit that Overlay might still not be the right fit but I think it's worth exploring further. Some other things to consider:
There was a problem hiding this comment. The programmatic approach is nifty, thanks for the example! I like the idea of using the CustomModal service to encapsulate common functionality like attaching the TemplatePortal and adding the backdropClick handler. I'll experiment with this method and re-request review when it's ready. There was a problem hiding this comment. The code has been updated to use Overlay! Note that Overlay features make the CustomModalComponent redundant; we can solely use the CustomModal service now. Some advantages of Overlay include:
I've added some convenience logic in CustomModal to make Overlays work well with our nested modals requirements, along with some new tests. PTAL! +cc @rileyajones P.S.
I've found that each overlay must specify its own separate position, which means that each uniquely positioned modal needs its own overlay. There was a problem hiding this comment. Excellent. Thanks for bearing with me. I'm glad it actually went somewhere. |
||
| constructor(private appRef: ApplicationRef) {} | ||
|
|
||
| /** Gets the ViewContainerRef of the app's root component if it exists. */ | ||
| private getModalViewContainerRef(): ViewContainerRef | undefined { | ||
| const appComponents = this.appRef.components; | ||
| if (appComponents.length === 0) { | ||
| // appComponents can be empty in tests. | ||
| return; | ||
| } | ||
|
|
||
| const appInstance = appComponents[0].instance; | ||
| let viewContainerRef: ViewContainerRef = appInstance.modalViewContainerRef; | ||
| if (!viewContainerRef) { | ||
| console.warn( | ||
| 'For proper custom modal function, an ViewContainerRef named `modalViewContainerRef` is required in the root component.' | ||
| ); | ||
| return; | ||
| } | ||
| return viewContainerRef; | ||
| } | ||
|
|
||
| /** Creates a modal using the given template at the given position. */ | ||
| createAtPosition( | ||
| templateRef: TemplateRef<unknown>, | ||
| position: {x: number; y: number} | ||
| ): CustomModalComponent | undefined { | ||
| const viewContainerRef = this.getModalViewContainerRef(); | ||
| if (!viewContainerRef) return; | ||
|
|
||
| const embeddedViewRef = viewContainerRef.createEmbeddedView(templateRef); | ||
| const modalComponent = CustomModalComponent.latestInstance; | ||
| modalComponent.parentEmbeddedViewRef = embeddedViewRef; | ||
| modalComponent.openAtPosition(position); | ||
| return modalComponent; | ||
| } | ||
|
|
||
| /** Triggers change detection for all modals in the view container. */ | ||
| runChangeDetection() { | ||
| const viewContainerRef = this.getModalViewContainerRef(); | ||
| if (!viewContainerRef) return; | ||
| for (let i = 0; i < viewContainerRef.length; i++) { | ||
| viewContainerRef.get(i)?.detectChanges(); | ||
| } | ||
| } | ||
|
|
||
| /** Destroys all modals in the view container. */ | ||
| closeAll() { | ||
| const viewContainerRef = this.getModalViewContainerRef(); | ||
| if (!viewContainerRef) return; | ||
| viewContainerRef.clear(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One relatively minor bug I noticed:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh great catch. There was some residual logic pertaining to the old implementation that was hiding the modal on the second click (
*ngIf="selectedFilter"). Removing this makes everything work as expected.