Skip to content

Conversation

@swift-kim
Copy link
Member

@swift-kim swift-kim commented Sep 13, 2021

Contributes to flutter-tizen/flutter-tizen#210.

Depends on flutter-tizen/engine#183 and flutter-tizen/engine#184 (AZURE_BUILD_ID=565).

tizen_app_control is now a separate package from the internal tizen_core package. The API is similar to tizen_core's, but the implementation, example, and integration_test have almost been re-written from scratch.

Notable API changes:

After publishing this package, we may:

  • Remove the AppControl implementation from tizen_core.
  • Update the multi app template.
  • Re-implement plugins:
    • share_plus_tizen
    • url_launcher_tizen
    • (image_picker_tizen)

Although this package is technically a pure Dart package, I decided to place it in this repo because it heavily depends on the embedder's native implementation.

@swift-kim swift-kim requested a review from a team September 13, 2021 07:31
@github-actions github-actions bot added the needs-publishing The package should be published after merge label Sep 13, 2021
@swift-kim
Copy link
Member Author

@HakkyuKim The CI build failed due to missing certificate profile. Can we add a certificate profile to the CI server?

@swift-kim
Copy link
Member Author

It would be important to note that the await AppControl.sendLaunchRequest call will not ever return when replyCallback is non-null and the callee app exited without replying (e.g. by pressing the back button in the image picker view). I couldn't find a good solution for this. @WonyoungChoi Do you know if it's an expected behavior on Tizen?

@swift-kim swift-kim force-pushed the add-tizen-app-control branch from ffe8d25 to d29a820 Compare September 13, 2021 08:41
@HakkyuKim
Copy link
Contributor

@HakkyuKim The CI build failed due to missing certificate profile. Can we add a certificate profile to the CI server?

#217 should solve the problem.

@swift-kim
Copy link
Member Author

@HakkyuKim Oops. I've already done the same thing in e3387a9. Should I revert?

@HakkyuKim
Copy link
Contributor

@swift-kim
Oh just saw it. I can close mine, let's just merge from this PR.

@HakkyuKim HakkyuKim mentioned this pull request Sep 13, 2021
Copy link
Contributor

@pkosko pkosko left a comment

Choose a reason for hiding this comment

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

Code looks very good. Approve from me.

@HakkyuKim
Copy link
Contributor

LGTM after solving:

It would be important to note that the await AppControl.sendLaunchRequest call will not ever return when replyCallback is non-null and the callee app exited without replying.

I also couldn't think of a right solution. If Tizen has no preferred way of handling this, maybe the API should document the fact so users can use Future.timeout?

@swift-kim
Copy link
Member Author

@HakkyuKim Thanks. I added some comments but didn't specify a preferred way to deal with the situation.

My personal recommendation is to use CancelableOperation from the async package with WidgetsBindingObserver (to listen for app resume events).

Copy link
Contributor

@HakkyuKim HakkyuKim left a comment

Choose a reason for hiding this comment

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

@swift-kim
Cool, didn't know that I could wrap a Future and cancel it. LGTM.

@swift-kim swift-kim merged commit 6a050df into flutter-tizen:master Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-publishing The package should be published after merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants