From c1304b700492f21b07e3405499148636084dcea4 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Wed, 8 Nov 2023 15:52:56 -0800 Subject: [PATCH] refactor(angular): popover controller uses correct core instance (#28485) Issue number: internal --------- ## What is the current behavior? As a takeaway from our learning session about a menuController bug in Ionic Angular, the team would like to update our other providers to use the same architecture as the menuController to prevent this kind of issue from happening again in the future. We also noticed that the common provider does not provide much value and it's easier to just have two separate implementations in src and standalone. (There wasn't much code we could de-duplicate) ## What is the new behavior? - Removed the common popover provider in favor of separate ones in src/standalone ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information I didn't add tests because there's already existing ones for popover controller. --- packages/angular/common/src/index.ts | 1 - packages/angular/common/src/utils/overlay.ts | 2 +- packages/angular/src/index.ts | 2 +- packages/angular/src/ionic-module.ts | 9 ++------ .../src/providers/popover-controller.ts | 21 +++++++++++++++++++ packages/angular/standalone/src/index.ts | 2 +- .../standalone/src/providers/ionic-angular.ts | 10 +++------ .../src/providers/popover-controller.ts | 8 ++----- 8 files changed, 31 insertions(+), 24 deletions(-) create mode 100644 packages/angular/src/providers/popover-controller.ts rename packages/angular/{common => standalone}/src/providers/popover-controller.ts (75%) diff --git a/packages/angular/common/src/index.ts b/packages/angular/common/src/index.ts index a2e611b0b4..971c6b268e 100644 --- a/packages/angular/common/src/index.ts +++ b/packages/angular/common/src/index.ts @@ -3,7 +3,6 @@ export { LoadingController } from './providers/loading-controller'; export { MenuController } from './providers/menu-controller'; export { ModalController } from './providers/modal-controller'; export { PickerController } from './providers/picker-controller'; -export { PopoverController } from './providers/popover-controller'; export { ToastController } from './providers/toast-controller'; export { AnimationController } from './providers/animation-controller'; diff --git a/packages/angular/common/src/utils/overlay.ts b/packages/angular/common/src/utils/overlay.ts index 1141eeb0da..7f9d0aa0fa 100644 --- a/packages/angular/common/src/utils/overlay.ts +++ b/packages/angular/common/src/utils/overlay.ts @@ -1,6 +1,6 @@ // TODO(FW-2827): types -interface ControllerShape { +export interface ControllerShape { create(options: Opts): Promise; dismiss(data?: any, role?: string, id?: string): Promise; getTop(): Promise; diff --git a/packages/angular/src/index.ts b/packages/angular/src/index.ts index 06b7e767b7..0a048c1cfc 100644 --- a/packages/angular/src/index.ts +++ b/packages/angular/src/index.ts @@ -24,7 +24,6 @@ export { LoadingController, ModalController, PickerController, - PopoverController, ToastController, AnimationController, GestureController, @@ -41,6 +40,7 @@ export { ViewDidLeave, } from '@ionic/angular/common'; export { MenuController } from './providers/menu-controller'; +export { PopoverController } from './providers/popover-controller'; export { ActionSheetController } from './providers/action-sheet-controller'; // PACKAGE MODULE diff --git a/packages/angular/src/ionic-module.ts b/packages/angular/src/ionic-module.ts index e8b0af6e2c..b4193e729f 100644 --- a/packages/angular/src/ionic-module.ts +++ b/packages/angular/src/ionic-module.ts @@ -1,12 +1,6 @@ import { CommonModule, DOCUMENT } from '@angular/common'; import { ModuleWithProviders, APP_INITIALIZER, NgModule, NgZone } from '@angular/core'; -import { - ModalController, - PopoverController, - ConfigToken, - AngularDelegate, - provideComponentInputBinding, -} from '@ionic/angular/common'; +import { ModalController, ConfigToken, AngularDelegate, provideComponentInputBinding } from '@ionic/angular/common'; import { IonicConfig } from '@ionic/core'; import { appInitialize } from './app-initialize'; @@ -29,6 +23,7 @@ import { IonModal } from './directives/overlays/modal'; import { IonPopover } from './directives/overlays/popover'; import { DIRECTIVES } from './directives/proxies-list'; import { IonMaxValidator, IonMinValidator } from './directives/validators'; +import { PopoverController } from './providers/popover-controller'; const DECLARATIONS = [ // generated proxies diff --git a/packages/angular/src/providers/popover-controller.ts b/packages/angular/src/providers/popover-controller.ts new file mode 100644 index 0000000000..96c5c8d5a8 --- /dev/null +++ b/packages/angular/src/providers/popover-controller.ts @@ -0,0 +1,21 @@ +import { Injector, inject, EnvironmentInjector } from '@angular/core'; +import { AngularDelegate, OverlayBaseController } from '@ionic/angular/common'; +import type { PopoverOptions } from '@ionic/core'; +import { popoverController } from '@ionic/core'; + +export class PopoverController extends OverlayBaseController { + private angularDelegate = inject(AngularDelegate); + private injector = inject(Injector); + private environmentInjector = inject(EnvironmentInjector); + + constructor() { + super(popoverController); + } + + create(opts: PopoverOptions): Promise { + return super.create({ + ...opts, + delegate: this.angularDelegate.create(this.environmentInjector, this.injector, 'popover'), + }); + } +} diff --git a/packages/angular/standalone/src/index.ts b/packages/angular/standalone/src/index.ts index 94e24efffe..eee11d2f09 100644 --- a/packages/angular/standalone/src/index.ts +++ b/packages/angular/standalone/src/index.ts @@ -6,13 +6,13 @@ export { IonRouterLink, IonRouterLinkWithHref } from './navigation/router-link-d export { IonTabs } from './navigation/tabs'; export { provideIonicAngular } from './providers/ionic-angular'; export { MenuController } from './providers/menu-controller'; +export { PopoverController } from './providers/popover-controller'; export { ActionSheetController } from './providers/action-sheet-controller'; export { AlertController, LoadingController, ModalController, PickerController, - PopoverController, ToastController, AnimationController, GestureController, diff --git a/packages/angular/standalone/src/providers/ionic-angular.ts b/packages/angular/standalone/src/providers/ionic-angular.ts index 8dd0053cc0..fc000dc58b 100644 --- a/packages/angular/standalone/src/providers/ionic-angular.ts +++ b/packages/angular/standalone/src/providers/ionic-angular.ts @@ -1,16 +1,12 @@ import { DOCUMENT } from '@angular/common'; import { APP_INITIALIZER } from '@angular/core'; import type { Provider } from '@angular/core'; -import { - AngularDelegate, - ConfigToken, - ModalController, - PopoverController, - provideComponentInputBinding, -} from '@ionic/angular/common'; +import { AngularDelegate, ConfigToken, ModalController, provideComponentInputBinding } from '@ionic/angular/common'; import { initialize } from '@ionic/core/components'; import type { IonicConfig } from '@ionic/core/components'; +import { PopoverController } from './popover-controller'; + export const provideIonicAngular = (config?: IonicConfig): Provider[] => { /** * TODO FW-4967 diff --git a/packages/angular/common/src/providers/popover-controller.ts b/packages/angular/standalone/src/providers/popover-controller.ts similarity index 75% rename from packages/angular/common/src/providers/popover-controller.ts rename to packages/angular/standalone/src/providers/popover-controller.ts index c4c7ccc725..837c7f647a 100644 --- a/packages/angular/common/src/providers/popover-controller.ts +++ b/packages/angular/standalone/src/providers/popover-controller.ts @@ -1,12 +1,8 @@ -import { Injector, Injectable, inject, EnvironmentInjector } from '@angular/core'; +import { Injector, inject, EnvironmentInjector } from '@angular/core'; +import { AngularDelegate, OverlayBaseController } from '@ionic/angular/common'; import type { PopoverOptions } from '@ionic/core/components'; import { popoverController } from '@ionic/core/components'; -import { OverlayBaseController } from '../utils/overlay'; - -import { AngularDelegate } from './angular-delegate'; - -@Injectable() export class PopoverController extends OverlayBaseController { private angularDelegate = inject(AngularDelegate); private injector = inject(Injector);