-
-
Notifications
You must be signed in to change notification settings - Fork 241
Update modal to allow use of ios presentationStyle #1771
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
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feat(modal): expose current/future core options via ModalDialogParams
This was referenced Mar 25, 2019
edusperoni
reviewed
Mar 25, 2019
MartoYankov
reviewed
Mar 26, 2019
chore(list-view): rename the new options API and change its type
MartoYankov
approved these changes
Mar 28, 2019
zbranzov
pushed a commit
that referenced
this pull request
Apr 23, 2019
zbranzov
pushed a commit
that referenced
this pull request
Apr 24, 2019
chore: add new test for #1771
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactored this and this PRs into a merged PR and also added "test case" to the modal-navigation-ng application for future e2e test.
PR Checklist
What is the current behavior?
From PR 1767
As per this issue #1709 it's currently not possible to use an ios popover style modal.
From PR 1769
The current exposed options for showModal differs from the options available in the core.
What is the new behavior?
From PR 1767
I've added a couple of new options to
ModalDialogOptions
viewContainerRef
but if you want to attach a popover to a button for example you need to pass in the button element.presentationStyle: UIModalPresentationStyle.Popover
, the only caveat with this is you will need to installtns-platform-declarations
to be able to use it.You will also see I've added a
closeModal()
option. There were 2 reasons for this. Firstly if you click on the background the popover disappears but when you try to open another popover you get an error warning a modal already exists. So at the moment if asourceView
is present it will close any pre existing modal first before opening a new one.The second issue I had is if I opened a popover then navigated back then went back to the same page to popover would reappear but not attached to the view it was previously attached to and with no content. So I made
closeModal()
public and call it when going back. i.eI'm sure there's maybe a better way to resolve these issues but wasn't sure on that.
This solutions probably isn't perfect but it does work without any issues as far as I can tell, so think it's a good starting point at least.
From PR 1769
All current and future non-angular options are exposed by leveraging Typescript's
Pick
type.Fixes/Implements/Closes
From PR 1767
#1709