Skip to content

Conversation

@ACTCD
Copy link
Collaborator

@ACTCD ACTCD commented Dec 9, 2022

resolved #395

In addition there are:

  • Fixed that "Can not write to path. Choose a different path." may be prompted when clicking the "Cancel" button in the path selection panel
  • Same as above, further ensuring processing is only done after the "open" button is clicked
  • Subsequent processing is only done if the new path is different from the current path
  • When opening the path selection panel, it will be positioned at the current Save Location by default
  • Ability to automatically refresh the extension popup and page after the path is indeed changed
  • Same as above, even if open directly from the App to change the path instead of the extension settings, can also auto-refresh

The newly added interaction improvements are:

  • Open the app directly from the extension without confirmation, there is no longer a redundant dialog box.
  • Since it is unavoidable to start Safari, after completing the location change when the app was opened before, switch to the extension page and return to the app.
  • If the app has not been opened before, when changing the location from the settings, the path selection panel will be opened directly without starting and rendering the main interface of the app, and the app will automatically exit after the selection is completed.
  • If the app has been opened before, when changing the location from the settings, the path selection panel will be called automatically, and the open state of the app will be kept after the selection is completed.
  • Some other minor modifications not listed
screenshot.mp4

@ACTCD ACTCD linked an issue Dec 9, 2022 that may be closed by this pull request
@ACTCD ACTCD marked this pull request as ready for review December 10, 2022 07:06
@ACTCD ACTCD requested a review from quoid December 10, 2022 07:06
Comment on lines 97 to 105
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))")
}
}
Copy link
Owner

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.

Copy link
Collaborator Author

@ACTCD ACTCD Dec 11, 2022

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.

@quoid
Copy link
Owner

quoid commented Dec 10, 2022

Great improvements, looks fantastic!

@ACTCD ACTCD marked this pull request as draft December 11, 2022 03:20
@ACTCD
Copy link
Collaborator Author

ACTCD commented Dec 11, 2022

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:
The current approach is to reload by refreshing the window in popup and page, which is very simple and avoids some potential problems. But a better approach may be to re-initialize the data through the front-end code, hot-update the interface, and even maintain the current view state, such as staying in the settings module. And achieve some kind of visual animation effect of the reloading process.

@ACTCD ACTCD changed the title Issue/395 Improve the interaction between app and extension Dec 20, 2022
@ACTCD ACTCD marked this pull request as ready for review December 24, 2022 07:39
@ACTCD ACTCD requested a review from quoid December 24, 2022 07:39
@ACTCD ACTCD merged commit 70bd9c3 into master Jan 26, 2023
@ACTCD ACTCD deleted the issue/395 branch January 26, 2023 17:40
Repository owner deleted a comment Jun 18, 2023
Repository owner deleted a comment Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize change save location experience

3 participants