search

maxgoedjen / secretive

Store SSH keys in the Secure Enclave
MIT License
7.24k stars 159 forks source link

Add option to rename keys/secrets #216

Closed diesal11 closed 3 years ago

diesal11 commented 3 years ago

Closes #215

diesal11 commented 3 years ago

No worries mate! It's a great project and a good excuse to get my hands dirty with some SwiftUI.

Thanks for the review, all issues have been addressed. The UI bug was a tricky one, it seems that using .constant for .popover(isPresenting: ...) is incorrect, as a popup can dismiss itself and the only way our views know is though an update to the binding passed via isPresenting.

Ideally we could just call .popover once for each type of popover and keep them completely seperate, but SwiftUI doesn't seem to be a fan of that idea and refuses to show any popups at all. So instead i've had to wrap it so a side effect can be performed when the popover is dismissed. Oh well.

maxgoedjen commented 3 years ago

Playing with this a bit more on my local Mac I'm still seeing some weird foreground/background stuff where the app is refusing to focus after bringing up the popovers – I'm not really sure if this is caused by anything wrong in this PR, just overall glitchiness with popovers in SwiftUI. I'll try and dig in a bit more to see if I can find the root cause.

diesal11 commented 3 years ago

I'm unable to reproduce any focus bugs running MacOS 11.3. I'd be happy to investigate if you could post a video reproducing the issue?

maxgoedjen commented 3 years ago

You can see it here: basically the window just instantly unfocuses itself. I'm still not sure that it's this PR but I haven't seen it on any other branch (and it seems inconsistent on this one fwiw – I tried to reproduce it earlier and was unable to).

https://user-images.githubusercontent.com/342665/120088623-69adc780-c0a7-11eb-979e-6907de7d2b21.mov

maxgoedjen commented 3 years ago

I think I'm seeing this on the prod version of Secretive as well now (#220) so I have no reason to think this is related to this PR. Please don't feel any obligation to fix this or anything, but if you have any ideas I'd for sure welcome them 🙏.

Thanks again for the PR and doubly so for your patience!

maxgoedjen commented 3 years ago

Just in case you were curious, it definitely wasn't related to this PR ;)

maxgoedjen commented 3 years ago

@diesal11 this is now out in 2.1.1, thanks for your contribution! (also found that other bug too 🌈)