FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Pass NS app to the native app instead of presenting it over the root VC by tdermendjiev · Pull Request #5967 · NativeScript/NativeScript · GitHub

feat: Pass NS app to the native app instead of presenting it over the root VC - #5967

Merged
tdermendjiev merged 9 commits into
masterfrom
tdermendzhiev/pass-embedded-ns-to-native
Jun 27, 2018
Merged

tdermendjiev merged 9 commits into
masterfrom
tdermendzhiev/pass-embedded-ns-to-native

Conversation

tdermendjiev commented Jun 19, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

When NativeScript embedded app is created from the native one we check for whether the topmost UIViewController has NativeScriptEmbedder protocol (implemented in the iOS Runtime) method 'presentNativeScriptApp:'. If yes, we call it with the NS app viewcontroller as a parameter so the embedder has control over the NS app (where and how to present it etc.) For backwards compatibility we present the NS app on top of the topmost UIViewController as a fallback.

PR Checklist

What is the current behavior?

Currently embedded app is always presented on top of the rootViewController.

What is the new behavior?

The embedder is now able to choose where to present the NS app (present, push, add as a child ViewController etc.) by implementing 'presentNativeScriptApp' method. Even when he does not implement it the NS app will present on top of the visible(topmost) viewcontroller instead of the root one (which may be hidden in some scenarios).

…enting it over the rootViewController

When NativeScript embedded app is created from the native one we check for whether the topmost UIViewController has NativeScriptEmbedder protocol (implemented in the iOS Runtime) method 'presentNativeScriptApp:'. If yes, we call it with the NS app viewcontroller as a parameter so the embedder has control over the NS app (where and how to present it etc.) For backwards compatibility we present the NS app on top of the topmost UIViewController as a fallback.
tdermendjiev self-assigned this Jun 19, 2018
ghost added the in progress label Jun 19, 2018
…:NativeScript/NativeScript into tdermendzhiev/pass-embedded-ns-to-native
tdermendjiev added a commit to NativeScript/ios-jsc that referenced this pull request Jun 19, 2018
When embedding a NativeScript application, the native class, calling the NS runtime, should conform to NativeScriptEmbedder protocol in order get a reference to the NativeScript application viewcontroller. Related to NativeScript/NativeScript#5967
tdermendjiev added a commit to NativeScript/ios-jsc that referenced this pull request Jun 19, 2018
When embedding a NativeScript application, the native class, calling the NS runtime, should conform to NativeScriptEmbedder protocol in order get a reference to the NativeScript application viewcontroller. Related to NativeScript/NativeScript#5967
tdermendjiev added a commit to NativeScript/ios-jsc that referenced this pull request Jun 19, 2018
When embedding a NativeScript application, the native class, calling the NS runtime, should conform to NativeScriptEmbedder protocol in order get a reference to the NativeScript application viewcontroller. Related to NativeScript/NativeScript#5967
tdermendjiev changed the title feat: Pass NS app to the native app instead of presenting it over the root VC WIP:feat: Pass NS app to the native app instead of presenting it over the root VC Jun 19, 2018
tdermendjiev changed the title WIP:feat: Pass NS app to the native app instead of presenting it over the root VC feat: Pass NS app to the native app instead of presenting it over the root VC Jun 20, 2018
tdermendjiev requested a review from mbektchiev June 20, 2018 10:19
}
}

public getTopmostViewController(rootViewController: UIViewController): UIViewController {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@vakrilov Do we need to add documentation about this new public function? It can be quite useful when dealing with UIViewControllers and I think that it's a good idea.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yup - we can expose it.
I think it better to make it static (it doesn't really need this). You can put it in the ios namespace in the utils module and the documentation should go in [utils.d.ts[(https://github.com/NativeScript/NativeScript/blob/master/tns-core-modules/utils/utils.d.ts#L185)

vakrilov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add docs for getVisibleViewController pls.
Otherwise looks good.

Comment thread tns-core-modules/utils/utils.d.ts Outdated
*/
export function getCurrentAppPath(): string;

export function getVisibleViewController(rootViewController: UIViewController): UIViewController;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Add JSDoc for getVisibleViewController

tdermendjiev force-pushed the tdermendzhiev/pass-embedded-ns-to-native branch 3 times, most recently from 447c5ef to 187f9a1 Compare June 25, 2018 14:53
NativeScriptEmbedder is responsive for communication between the NS and the native iOS app. His delegate will implement methods which we can call from javascript such as "presentNativeScriptApp:".
mbektchiev added this to the 4.2 (TBD) milestone Jun 27, 2018
tdermendjiev merged commit 05c2460 into master Jun 27, 2018
tdermendjiev deleted the tdermendzhiev/pass-embedded-ns-to-native branch June 27, 2018 13:48
mbektchiev added the ready for test TSC needs to test this and confirm against live production apps and automated test suites label Jun 28, 2018

lock Bot commented Aug 26, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Aug 26, 2019
This file contains hidden or 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes ready for test TSC needs to test this and confirm against live production apps and automated test suites

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL