Skip to content

🔧 Improved Accessibility Permissions handling#77

Merged
the0neyouseek merged 6 commits intomasterfrom
quitafterperms
May 23, 2019
Merged

🔧 Improved Accessibility Permissions handling#77
the0neyouseek merged 6 commits intomasterfrom
quitafterperms

Conversation

@JoniVR
Copy link
Copy Markdown
Member

@JoniVR JoniVR commented May 8, 2019

Small Quality of Life change.

Currently when the app shows the popup asking for accessibility permissions, the app doesn't work right after granting these permissions because the app is still launched with previous permissions (would be my guess).

This just makes sure the app exits after asking for permissions and tells the user to restart manually after setting permissions. I think it's a bit more user friendly, as the app doesn't work without a manual restart after granting permissions, but let me know what you think.

I looked into somehow listening for this setting but I don't think it's possible.

Also, you might want to proofread those French and German translations, since I used DeepL for translations

edit:
Permissions now work once the user grants them without having to manually restart the app.

@JoniVR JoniVR changed the title quits app after asking for accessibility permissions quit app after asking for accessibility permissions May 8, 2019
Comment thread MonitorControl/Support/de.lproj/Localizable.strings Outdated
@JoniVR JoniVR requested a review from the0neyouseek May 8, 2019 19:18
@the0neyouseek
Copy link
Copy Markdown
Member

I don't know if we should do this, making the app unusable unless the user does an action isn't really great in an usability stand point.
Maybe we should simply put the message "Restart when done" like you did and add a button that restart the app on the menulet with a message (like chrome does for their updates). Something like ":warning: Accessibility permissions required. Click to restart when enabled"

@JoniVR
Copy link
Copy Markdown
Member Author

JoniVR commented May 8, 2019

I agree, that's a better solution.

@JoniVR
Copy link
Copy Markdown
Member Author

JoniVR commented May 9, 2019

example
Any suggestions for a shorter message? Or a different solution? I think this looks a bit too weird lol.

I was thinking maybe "⚠ Accessibility Permissions - Restart Now." but that doesn't really pass the message along well enough. Or just "⚠ Restart Now."

@the0neyouseek
Copy link
Copy Markdown
Member

the0neyouseek commented May 10, 2019

Can we detect if permissions were changed ? In that case it could be ⚠ Acessibility permissions required at first, and after enabling⚠ Permissions changed, restart now

If not I guess simply ⚠ Restart app now will do

@JoniVR
Copy link
Copy Markdown
Member Author

JoniVR commented May 10, 2019

Can we detect if permissions were changed ?

I initially thought this was impossible, but after doing some deep digging I think I found out a way to do this. It's almost entirely undocumented but some apps (like Magnet) do this already.

Apparently, you can just observe the System Preferences "com.apple.accessibility.api" distributed notification. (source)

like this:

DistributedNotificationCenter.default().addObserver(self, selector: #selector(self.readPrivileges), name: NSNotification.Name(rawValue: "com.apple.accessibility.api"), object: nil)

to detect a change in Accessibility permissions for your applications.

edit: I'm currently having an issue with this sometimes locking up the system.. looking into it.

Magnet.app even manages to not require a restart at all after changing permissions.

So I'm trying to figure out how to do this too. This way we'd be able to show a warning with a button to set accessibility permissions in the menulet and not have to restart when the user grants the permission. Which I think would be most ideal.

You can easily test this on Magnet if you have it, if you revoke their permissions their menulet shows:
image

Once you authorize, it just works!

So either we only use this observer to detect the initial change and then remove it or we can keep it in there in case the user for some reason removes the privilege while the app is running.

@JoniVR JoniVR changed the title quit app after asking for accessibility permissions Improved Accessibility Permissions handling May 16, 2019
JoniVR added 2 commits May 18, 2019 23:36
- restart mediakeytap after changing accessibility permissions
- add accessibility observer
@JoniVR
Copy link
Copy Markdown
Member Author

JoniVR commented May 22, 2019

Ok so I figured out what caused the freezing (was something really stupid), right now it works pretty well. No restarting required at all, It just restarts mediakeytap and that's enough, then it just works, I don't know how I overlooked that..

So if the user grants permission it will automatically start working without restarting the app.

This is ready for testing/merging. No need for translations since no buttons were added.

- Add an extension to NSNotification.Name
- Fix whitespaces
@the0neyouseek the0neyouseek changed the title Improved Accessibility Permissions handling 🔧 Improved Accessibility Permissions handling May 23, 2019
@the0neyouseek the0neyouseek merged commit 4dddd27 into master May 23, 2019
@the0neyouseek the0neyouseek deleted the quitafterperms branch May 23, 2019 10:35
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.

3 participants