Ionic-framework: feat: pass T in onDidDismiss to OverlayEventDetail

Created on 25 May 2020  路  9Comments  路  Source: ionic-team/ionic-framework

Feature Request

the onDidDismiss, e.g. for a Modal, returns OverlayEventDetail<any>.
The any is poor for coding, it would be better to be able to pass T.
Right now onDidDismiss is defined like so:
"onDidDismiss": () => Promise<OverlayEventDetail<any>>;
please change to
"onDidDismiss": <T = any>() => Promise<OverlayEventDetail<T>>;

so T is still optional, but if you want, you can have it typed now.

Ionic version:


[x] 4.x

Describe the Feature Request

Describe Preferred Solution

Describe Alternatives

Related Code

Additional Context

help wanted core feature request

All 9 comments

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!

@liamdebeasi Hello, I would like to take this. May I do this issue?

Yes, thank you!

Here is our contributing guide that gives you an overview of how to make the changes, test, and create a PR: https://github.com/ionic-team/ionic/blob/master/.github/CONTRIBUTING.md#creating-a-pull-request.

Let me know if you have any questions 馃槉.

Nice, didn鈥榯 know that this is possible. Next time i contribute right away :)

@aivinog1 maybe you can look out for other locations where it鈥榮 just any and change to T=any so we have the possibility to type it.

@nickwinger Yes, I'll try to find such methods, but if there are too many such methods I think that the best solution is to split such PR to many.

In the first iteration, I can find all the possible places and describe them in the comments of this task.

Thanks for the feature request. This has been added via https://github.com/ionic-team/ionic/pull/21393 and will be available in an upcoming release of Ionic Framework.

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.

Was this page helpful?
0 / 5 - 0 ratings