Skip to content
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

fix(permissions): macos ux improvement #1279

Open
wants to merge 62 commits into
base: main
Choose a base branch
from

Conversation

oliverqx
Copy link
Contributor

@oliverqx oliverqx commented Feb 6, 2025

/claim #1273
/claim #1264

description

this pr will address the following issues: #1273, #1264

goals

  • implement permission dialog
  • improve onboarding lifecycle, so it reopens after restarting when permissions are granted. (fixed with feat(tauri): add prod configuration file #1349)
  • fix permissions showing green when toggled off
  • fix permission showing green when permission was given to a dev build
  • hide permission related interfaces form windows/linux
  • fix mac os unlinking permissions between updates/rebuilds ([bounty] fix macos permission UI #1264)

Copy link

vercel bot commented Feb 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 11, 2025 9:26pm

@louis030195
Copy link
Collaborator

louis030195 commented Feb 7, 2025

why is there 26 file changes?

this should be like 3-4 files not more

keep it simple

@oliverqx
Copy link
Contributor Author

oliverqx commented Feb 7, 2025

@louis030195 ik.. most of those changes are just reorganizing files for better dx:

  1. moved all slides into @/components/onboarding/slides
  2. moved use-onboarding to @/components/onboarding/context
  3. cleaned up context by creating three hooks in @/components/onboarding/hooks
  4. moved flow to its own file
  5. implemented framer motion for slide animation

Its just that while debugging onboarding it was hard to understand its flow, lots of places controlled dialog visibility and user input state. For example, showOnboarding was being set in three different components for different reasons which made onboarding not reopen consistently upon restarting the app

core logic remains the same

with the changes its now really easy to understand when and why showOnboarding is set to false, dialog will reopen at permissions slide when restarting for screen recording permissions

return type of permissions is string, always true. check should be similar to onboarding's status
- moved file into @/components/onboarding
- renamed onboarding.tsx to dialog.tsx
final result @/components/onboarding/dialog.tsx
- move into @/components/onboarding
- rename from use-onboarding to context
final result: @/components/onboarding/context
- move all slides from @/components/onboarding to @/components/onboarding/slides
…ons after "request_permission" is invoked

this gives user and system time to grant permissions before we trigger a check. Otherwise, function checks prematurely
- use localforage to set showOnboarding
- make functions asynchronous
Copy link

vercel bot commented Feb 12, 2025

@oliverqx is attempting to deploy a commit to the louis030195's projects Team on Vercel.

A member of the Team first needs to authorize it.

@oliverqx oliverqx marked this pull request as ready for review February 14, 2025 00:50
@oliverqx
Copy link
Contributor Author

oliverqx commented Feb 14, 2025

@louis030195 I think this is ready to be reviewed. I know its a lot but here's a quick overview of what I've done:

  1. moved all slides into @/components/onboarding/slides
  2. moved use-onboarding to @/components/onboarding/context
  3. cleaned up context by creating three hooks in @/components/onboarding/hooks
  4. moved flow to its own file
  5. implemented framer motion for slide animation
  6. created use-permissions hook
  7. created screenpipe status context
  8. moved use-health-check into screenpipe status context

especially now that use-health-check is a webhook it's best to keep it in a context instead of mounting the webhook everytime useHealthCheck is called. This way only one webhook is active and receiving updates, all other components can grab both health and permissions information from this context

  1. moved screenpipe status dialog from the status badge component to main page
  2. implemented useEffect to have screenpipe status context open status dialog if permissions are broken or health is compromised
  3. updated health_check in rust backend as it was not giving accurate info about audio (f06a2ab)

id be down to schedule a quick meeting to go over the changes if necessary

restart is handled in useOnboardingVisibility hook
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.

2 participants