Skip to content

Conversation

@swift-kim
Copy link
Member

Part of the code refactoring project.

app_settings_manager.cc:

  • Remove the AppPermissions class and move its logic to AppSettingsManager::OpenAppSettings. The class has been first introduced in [permission_handler] Revise OpenAppSettings #142, but adds unnecessary complexity to the code.
  • Replace the PackageName class with a simple function GetPackageName.

permission_handler_tizen_plugin.cc & permission_manager.cc:

  • The code has almost been rewritten from scratch (reduced 200~ lines of code).
  • The PermissionManager class is a copy of image_picker_tizen's PermissionManager. (See [image_picker] Refactor the C++ code #353.)
    • The only difference is that PermissionStatus and PermissionResult are now a single type, in accordance with the type defined in the platform interface package.
  • Rename PermissionGroup to Permission.
  • Remove all on_success and on_error callbacks as everything is now done synchronously.
  • Iterating over requested permissions is done inside HandleMethodCall's if-else block.
  • Do not redundantly check for the permission status before requesting it.
  • Remove duplication of kLocation results as it's not expected by anybody.

service_manager.cc:

  • Make ServiceManager::CheckServiceStatus synchronous.

@swift-kim swift-kim requested review from HakkyuKim and bbrto21 May 10, 2022 03:00
@swift-kim swift-kim force-pushed the refactor-permission-handler branch from b4da809 to 74a0e2c Compare May 10, 2022 05:02
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.

Looks nice! LGTM.

@swift-kim swift-kim force-pushed the refactor-permission-handler branch from 74a0e2c to dd896d6 Compare May 11, 2022 07:37
@swift-kim
Copy link
Member Author

Let me test this change on TV tomorrow. Now this plugin is expected to work on TV (although not necessary because all permissions are already granted) and we don't have to explicitly exclude the permission_handler dependency from other plugins tests.

@swift-kim
Copy link
Member Author

Tested on a TV emulator and it worked.

@swift-kim swift-kim merged commit d47b635 into flutter-tizen:master May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants