-
-
Notifications
You must be signed in to change notification settings - Fork 865
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
Added macOS implementation #946
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #946 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 16 16
=========================================
Hits 16 16 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Hello, |
Is it possible to add feature to request full disk access? |
me also waiting for this pr ,, mac os permissions is a necessary for any flutter app that is looking to support macOS platform so I think it should be a priority . |
Would also appreciate this a lot. |
Until this is merged, you can use:
|
I also vote for this feature! |
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.
First of all, apologies for the very late reply. Due to shifting priorities and under staffing we unfortunately had to postpone our work on the open source plugins. Luckily we have resolved the staffing issue and made the open source plugins a high priority.
I really appreciate the effort put into this PR and I am hoping you would be able to make some changes so the macOS support for the permission_handler is inline with support for other plugins we host (e.g. https://github.com/baseflow/flutter-geolocator). The two major request would be to:
- Create the macOS project as an Objective-C project instead of using the Swift language. We are aware Swift is a more developer friendly language, however by adding Swift to the project now adds another language to the project that developers need to understand and maintain. Since the iOS version is fully coded in Objective-C we'd prefer the macOS support to also be written in Objective-C.
- Currently most of the code is duplicated between the iOS and the macOS project. For the geolocator project we use sym-links in the macOS project reusing the source code files from the iOS project in the macOS project. This way we only have to maintain the code in one place. Use preprocessor directives to add iOS or macOS specific code blocks:
#if TARGET_OS_OSX
#import <FlutterMacOS/FlutterMacOS.h>
#else
#import <Flutter/Flutter.h>
#endif
Please let me know if you are up for bringing this PR to a point we can merge it into the main repository. If not we would be happy to take over the PR and apply the necessary changes.
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 plugin maintains a .gitignore
file in the root of the repository, therefore this file is not necessary and would add extra maintenance effort. Could you remove it from the PR?
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.
Please revert this file as it should not be related to adding macOS support.
This is most likely automatically generated by Flutter but I'd prefer to have this updated in a separate PR.
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.
Please revert this file, we maintain a version of the analysis_options.yaml
in the root of the repository. This file will override the setting configured there.
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.
Please revert this file, we maintain a version of the analysis_options.yaml in the root of the repository. This file will override the setting configured there.
<key>NSAppleMusicUsageDescription</key> | ||
<string>Music!</string> | ||
<key>NSBluetoothAlwaysUsageDescription</key> | ||
<string>bluetooth</string> | ||
<key>NSBluetoothPeripheralUsageDescription</key> | ||
<string>bluetooth</string> | ||
<key>NSCalendarsUsageDescription</key> | ||
<string>Calendars</string> | ||
<key>NSCameraUsageDescription</key> | ||
<string>camera</string> | ||
<key>NSContactsUsageDescription</key> | ||
<string>contacts</string> | ||
<key>NSLocationAlwaysAndWhenInUseUsageDescription</key> | ||
<string>Always and when in use!</string> | ||
<key>NSLocationAlwaysUsageDescription</key> | ||
<string>Can I have location always?</string> | ||
<key>NSLocationUsageDescription</key> | ||
<string>Older devices need location.</string> | ||
<key>NSLocationWhenInUseUsageDescription</key> | ||
<string>Need location when in use</string> | ||
<key>NSMicrophoneUsageDescription</key> | ||
<string>microphone</string> | ||
<key>NSMotionUsageDescription</key> | ||
<string>motion</string> | ||
<key>NSPhotoLibraryUsageDescription</key> | ||
<string>photos</string> | ||
<key>NSRemindersUsageDescription</key> | ||
<string>reminders</string> | ||
<key>NSSpeechRecognitionUsageDescription</key> | ||
<string>speech</string> | ||
<key>NSUserTrackingUsageDescription</key> | ||
<string>appTrackingTransparency</string> |
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.
Any specific reason why the comments have been removed? They were added on purpose for developers looking at this file to understand how permissions are mapped to the PermissionGroup
enum.
If there is not specific reason for removing the comments, please rollback this change.
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 plugin maintains a .gitignore file in the root of the repository. The **/Flutter/ephemeral/
and **/dgph
entries are missing however.
Please add the missing entries in the root .gitignore
file and revert this copy.
Hello @mvanbeusekom the work I did here was barebones since I was required to implement these permissions in a company's project. The reason I chose Swift was because
I naively thought that I could help other people by sharing this PR, but I'm realising that most of the changes I did were a bit rough. Right now I lack the time and the experience to do the work requested, so if you could please take in charge the PR it would be great. Thank you. |
This PR seems to aim to close #337. |
Please, push this feature. 🙏🏼 |
@bvoq are you able to update your README? i tried to add it in my pubspec.yaml but it seems it's not properly installed coz it's not found when i imported it. |
@hpelitebook745G2 It works fine if you import it directly via Github using the method I described above. You won't be able to download the plugin and use it offline without making some small changes. If you want to host it yourself, you will have to change all the |
Any updates? |
✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)
This PR adds the macOS implementation of the plugin.
Currently it doesn't support macOS.
🆕 What is the new behavior (if this is a feature change)?
Now it supports macOS permissions.
💥 Does this PR introduce a breaking change?
No.
🐛 Recommendations for testing
A couple of entitlements won't work as there's no equivalent to be found in macOS.
Some features work only from macOS 10.12, which is the minimum version that Flutter supports.
📝 Links to relevant issues/docs
No.
🤔 Checklist before submitting
master
.