Skip to content

Refactor modal options to expose current/future core options#1769

Closed
edusperoni wants to merge 2 commits into
NativeScript:masterfrom
edusperoni:ios-popover
Closed

Refactor modal options to expose current/future core options#1769
edusperoni wants to merge 2 commits into
NativeScript:masterfrom
edusperoni:ios-popover

Conversation

@edusperoni
Copy link
Copy Markdown
Collaborator

PR Checklist

What is the current behavior?

The current exposed options for showModal differs from the options available in the core.

What is the new behavior?

All current and future non-angular options are exposed by leveraging Typescript's Pick type.

#1767 attempts to fix this, but another property has been exposed (android: any), so other PRs would be needed for every new property.

I'm asking for this PR to be considered over #1767

Fixes #1709.

@VladimirAmiorkov
Copy link
Copy Markdown
Contributor

VladimirAmiorkov commented Mar 22, 2019

Hi @edusperoni ,

Thank you for your contribution with this PR and the comments on the #1767 PR. As I see that your PR's code changes have been taken and added to the #1767 PR and as it also contains additional functionality in the form of the new sourceView property of the ModalDialogOptions that is required for some of the ios specific options like the popover to work I suggest we merge that PR and close this one.

@edusperoni
Copy link
Copy Markdown
Collaborator Author

I don't really care which PR is merged, but that PR is still using the any cast that should be removed and the commits are kind of messy.

@VladimirAmiorkov
Copy link
Copy Markdown
Contributor

@edusperoni yes I have noticed that, it also is missing a "test" case example. We are going to process it.

@VladimirAmiorkov
Copy link
Copy Markdown
Contributor

Closing in favor of #1771

@ghost ghost removed the ♥ community PR label Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

modal presentationStyle

3 participants