Skip to content

Commit

Permalink
refactor(angular): picker controller uses correct core instance (#28521)
Browse files Browse the repository at this point in the history
Issue number: Internal

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## 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?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Removed the common picker provider in favor of separate ones in
src/standalone

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
  • Loading branch information
mapsandapps authored Nov 16, 2023
1 parent f143bd0 commit 9d57758
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 10 deletions.
1 change: 0 additions & 1 deletion packages/angular/common/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export { AlertController } from './providers/alert-controller';
export { LoadingController } from './providers/loading-controller';
export { MenuController } from './providers/menu-controller';
export { PickerController } from './providers/picker-controller';
export { DomController } from './providers/dom-controller';
export { NavController } from './providers/nav-controller';

Expand Down
2 changes: 1 addition & 1 deletion packages/angular/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export * from './directives/validators';
export {
AlertController,
LoadingController,
PickerController,
DomController,
NavController,
Config,
Expand All @@ -40,6 +39,7 @@ export { ActionSheetController } from './providers/action-sheet-controller';
export { GestureController } from './providers/gesture-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';

Expand Down
13 changes: 13 additions & 0 deletions packages/angular/src/providers/picker-controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Injectable } from '@angular/core';
import { OverlayBaseController } from '@ionic/angular/common';
import type { PickerOptions } from '@ionic/core';
import { pickerController } from '@ionic/core';

@Injectable({
providedIn: 'root',
})
export class PickerController extends OverlayBaseController<PickerOptions, HTMLIonPickerElement> {
constructor() {
super(pickerController);
}
}
2 changes: 1 addition & 1 deletion packages/angular/standalone/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ export { AnimationController } from './providers/animation-controller';
export { GestureController } from './providers/gesture-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 {
AlertController,
LoadingController,
PickerController,
DomController,
NavController,
Config,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { Injectable } from '@angular/core';
import { OverlayBaseController } from '@ionic/angular/common';
import type { PickerOptions } from '@ionic/core/components';
import { pickerController } from '@ionic/core/components';

import { OverlayBaseController } from '../utils/overlay';
import { defineCustomElement } from '@ionic/core/components/ion-picker.js';

@Injectable({
providedIn: 'root',
})
export class PickerController extends OverlayBaseController<PickerOptions, HTMLIonPickerElement> {
constructor() {
super(pickerController);

// TODO: FW-5415 may remove the need for this
defineCustomElement();
}
}
6 changes: 6 additions & 0 deletions packages/angular/test/base/e2e/src/lazy/providers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,11 @@ describe('Providers', () => {

cy.get('ion-action-sheet').should('be.visible');
});

it('should open a picker', () => {
cy.get('button#open-picker').click();

cy.get('ion-picker').should('be.visible');
});
});

Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
describe('Overlay Controllers', () => {
beforeEach(() => {
cy.visit('/standalone/overlay-controllers');
})
});

it('should present a modal', () => {
cy.get('button#open-modal').click();

cy.get('ion-modal app-dialog-content').should('be.visible');
});

it('should present a picker', () => {
cy.get('button#open-picker').click();

cy.get('ion-picker .picker-button').should('be.visible');
});

it('should present a popover', () => {
cy.get('button#open-popover').click();

cy.get('ion-popover app-dialog-content').should('be.visible');
});
})
});
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@

<button id="set-menu-count" (click)="setMenuCount()">Set Menu Count</button>
<button id="open-action-sheet" (click)="openActionSheet()">Open Action Sheet</button>
<button id="open-picker" (click)="openPicker()">Open Picker</button>
</ion-content>
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class ProvidersComponent {
alertCtrl: AlertController,
loadingCtrl: LoadingController,
private menuCtrl: MenuController,
pickerCtrl: PickerController,
private pickerCtrl: PickerController,
modalCtrl: ModalController,
platform: Platform,
popoverCtrl: PopoverController,
Expand Down Expand Up @@ -96,4 +96,18 @@ export class ProvidersComponent {

await actionSheet.present();
}

async openPicker() {
const picker = await this.pickerCtrl.create({
columns: [],
buttons: [
{
text: 'Cancel',
role: 'cancel',
},
]
});

await picker.present();
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<div>
<button id="open-modal" (click)="openModal()">Open Modal</button>
<button id="open-picker" (click)="openPicker()">Open Picker</button>
<button id="open-popover" (click)="openPopover($event)">Open Popover</button>
</div>
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { Component } from '@angular/core';
import { ModalController, PopoverController } from '@ionic/angular/standalone';
import { ModalController, PickerController, PopoverController } from '@ionic/angular/standalone';

@Component({
selector: 'app-overlay-controllers',
templateUrl: './overlay-controllers.component.html',
standalone: true,
})
export class OverlayControllersComponent {
constructor(private modalCtrl: ModalController, private popoverCtrl: PopoverController) {}
constructor(
private modalCtrl: ModalController,
private pickerCtrl: PickerController,
private popoverCtrl: PopoverController
) {}

async openModal() {
const modal = await this.modalCtrl.create({
Expand All @@ -17,6 +21,20 @@ export class OverlayControllersComponent {
await modal.present();
}

async openPicker() {
const picker = await this.pickerCtrl.create({
columns: [],
buttons: [
{
text: 'Cancel',
role: 'cancel',
},
],
});

await picker.present();
}

async openPopover(ev: MouseEvent) {
const popover = await this.popoverCtrl.create({
component: DialogComponent,
Expand Down

0 comments on commit 9d57758

Please sign in to comment.