-
Notifications
You must be signed in to change notification settings - Fork 1
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
Hybrid-Native Bridging Implementation PoC #1
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I get the main gist of this POC. My first reaction to this was that it's difficult to understand the flow of the code, so my thoughts are mainly toward how to make this communication as less awkward as possible. I was a little confused by the update(_:)
and displayError(_:)
functions but as far as I understand maybe we won't need these? In other words, do we foresee any other hybrid-->native calls beside "openBook" for our plugin? I suppose displayError
could be useful for hybrid to communicate an external error situation to the reader.
But at a high level, R2Plugin
is opened from the hybrid side when the user opens a book, and from there the native side takes control and can initiate a number of native --> hybrid actions (add/remove bookmarks, etc). Is this correct?
To me it sort of feels like R2Plugin
is a wrapper of the CAPPluginCall
value it's first called with by the hybrid side. And this "Reader Plugin" can initiate a series of actions on its own (createSampleModel
, updateSampleModel
, etc).
] | ||
|
||
DispatchQueue.main.async { | ||
self.bridge?.viewController?.present(self.module.viewController, animated: true, completion: nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will present the module
VC modally. Do you know if it's possible to push a new VC on the current navigator controller stack? something like
self.bridge?.viewController?.navigationController
.pushViewController(self.module.viewController)
I just don't know if Capacitor bridge view controller lives in a navigator controller we can access/control. If we can only present native views modally, that would suck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this. Although since self.bridge?.viewController
is a regular UIViewController, I think it should be possible to extend it however we see fit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great question though, and I will try and see if I can find any documentation on this / test it out locally
return | ||
} | ||
|
||
let models: [SampleModel] = array.compactMap { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this conversion would be best done with custom Codable types.
extension R2Plugin: SampleModuleDelegate { | ||
|
||
func createSampleModel(sampleModel: SampleModel) { | ||
pluginReceiver?.resolve([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in order for this to work, openBook
must be called first so that pluginReceiver
can be set, so there are some race conditions here that we will need to watch. I wonder if we can structure this in a way to avoid the expectation that pluginReceiver
has to be set by someone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, it would be safe to assume that the hybrid catalog will always be the one to initiate the contact with the native module -- meaning the setup call can be expected to occur before any updates are made (also the UI that would trigger these operations would not be accessible without a plugin to first setup / present it)
We may not need this for the first milestone, but I foresee the need for the hybrid app to send updates to the native app. For instance, if the native reader calls hybrid catalog to make an authenticated call, the result of the call would be passed through the update function |
FYI @jkim-git and @ettore there is an adapted version of this code with working bookmarks here: https://github.com/NYPL-Simplified/circulation-patron-web/pull/530 |
This pull request contains the native (iOS) implementation of Capacitor Plugin, showcasing:
Not yet implemented, planned tasks: