-
Notifications
You must be signed in to change notification settings - Fork 360
Improve the interaction between app and extension #404
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
Conversation
| SFSafariApplication.dispatchMessage( | ||
| withName: "SAVE_LOCATION_CHANGED", | ||
| toExtensionWithIdentifier: self.extensionID, | ||
| userInfo: ["saveLocation": url.absoluteString.removingPercentEncoding ?? "??"] | ||
| ) { error in // always be called | ||
| if error != nil { | ||
| debugPrint("Message attempted. Error info: \(String.init(describing: error))") | ||
| } | ||
| } |
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.
Right now when the user changes the save location, the manifest still refers to the old save location content., but it would be nice to also run the manifest cleanup/update process once we know the save location was successfully changed. The same code that runs on popupInit.
This is not a blocker and I think this could be done in a different PR. I thought of this when reviewing and wanted to comment on it.
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.
Good point. If you want to reuse those codes, some structural adjustments may be required.
In addition, we may eventually migrate most of the data from manifest to storage, which also needs to be considered.
|
Great improvements, looks fantastic! |
|
Due to some new ideas, and the need for more time to implement them, I turned this PR to Draft again. Another related improvement that I didn't make, neither intends to achieve, but I'd like to document it: |
resolved #395
In addition there are:
Save Locationby defaultpopupandpageafter the path is indeed changedThe newly added interaction improvements are:
screenshot.mp4