Skip to content
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

feat: add strong typing for ModalController dismiss values #20088

Closed
RoopeHakulinen opened this issue Dec 15, 2019 · 4 comments
Closed

feat: add strong typing for ModalController dismiss values #20088

RoopeHakulinen opened this issue Dec 15, 2019 · 4 comments
Labels
help wanted a good issue for the community package: core @ionic/core package type: feature request a new feature, enhancement, or improvement

Comments

@RoopeHakulinen
Copy link

Feature Request

Ionic version:
[x] 4.x

Describe the Feature Request
Currently the dismissal data of a shown modal is not typed. onWillDismiss uses the following signature:

'onWillDismiss': () => Promise<OverlayEventDetail<any>>;

OverlayEventDetail already supports the parametrization but any type is specified here. Instead it could be using a type specified when creating the modal so this

const modal = await this.modalController.create({
  component: EditDeviceSettingsPage
});
await modal.present();
const { settings } = await modal.onWillDismiss().then(({ data }) => data);
// settings is of type any

would become

const modal = await this.modalController.create<Settings>({
  component: EditDeviceSettingsPage
});
await modal.present();
const { settings } = await modal.onWillDismiss().then(({ data }) => data);
// settings is of type Settings

Describe Preferred Solution
Add type parameter for ModalController::create to specify the possible value returned upon dismissal.

Additional Context
This would not improve the typing in the modal component itself as it could still dismiss anything it wishes. I cannot foresee any chance to improve that with the current design but this would at least improve the ergonomics on the parent.

@ionitron-bot ionitron-bot bot added the triage label Dec 15, 2019
@brandyscarney brandyscarney added help wanted a good issue for the community package: core @ionic/core package type: feature request a new feature, enhancement, or improvement labels Dec 16, 2019
@ionitron-bot
Copy link

ionitron-bot bot commented Dec 16, 2019

This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.

If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.

For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.

Thank you!

@ionitron-bot ionitron-bot bot removed the triage label Dec 16, 2019
@tawfiek
Copy link
Contributor

tawfiek commented Feb 24, 2020

Interested .. I will investigate in this

@brandyscarney
Copy link
Member

Thank you for the issue! This has been done with the following PR: #21393

Please let me know if this doesn't solve your use case and I will reopen, thanks!

@ionitron-bot
Copy link

ionitron-bot bot commented Jul 4, 2020

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted a good issue for the community package: core @ionic/core package type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

No branches or pull requests

4 participants
@RoopeHakulinen @brandyscarney @tawfiek and others