Skip to content

fix(onboarding): stop duplicate permission prompts (v1.14.3)#37

Merged
rmarinsky merged 1 commit into
mainfrom
release/v1.14.3-permission-fixes
May 15, 2026
Merged

fix(onboarding): stop duplicate permission prompts (v1.14.3)#37
rmarinsky merged 1 commit into
mainfrom
release/v1.14.3-permission-fixes

Conversation

@rmarinsky

@rmarinsky rmarinsky commented May 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • Microphone permission now branches on authorizationStatus: opens the Privacy → Microphone pane on denied/restricted instead of waiting silently.
  • Accessibility permission: if grant is still missing after the native prompt, deep-link to Privacy → Accessibility.
  • Screen recording: removed the duplicate NSAlert in ensureScreenRecordingPermission; we now call requestScreenRecordingPermission directly. The macOS TCC prompt is enough — stacking a custom alert on top was the "feels triggered twice" bug.
  • Buttons: added contentShape so the entire pill is hit-testable (not just label glyphs).

Test plan

  • Reset TCC: tccutil reset Microphone ua.com.rmarinsky.diduny && tccutil reset Accessibility ua.com.rmarinsky.diduny && tccutil reset ScreenCapture ua.com.rmarinsky.diduny
  • Launch onboarding → microphone card → tap Allow → grant in prompt → auto-advance.
  • Tap Deny on microphone prompt → System Settings opens to Privacy → Microphone.
  • Accessibility card → Allow → check that no double prompt appears.
  • Screen Recording card → only one system prompt, no extra Diduny alert.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed duplicate system prompts when requesting screen recording permission during setup
    • Improved microphone permission request flow with better state handling
    • Enhanced accessibility permission dialog to display correctly when access is needed
    • Increased clickable area size for onboarding action buttons for better usability

Review Change Stack

…vacy pane on deny

- Microphone: branch on AVCaptureDevice.authorizationStatus — open the
  Privacy_Microphone pane when denied instead of waiting silently.
- Accessibility: if grant is still missing after the native prompt,
  deep-link to Privacy_Accessibility so the user has a visible next step.
- Screen recording: drop the duplicate NSAlert in
  ensureScreenRecordingPermission and call requestScreenRecordingPermission
  directly — macOS already shows the 'Open System Settings' prompt and the
  extra alert stacked two pop-ups (the 'feels triggered twice' bug).
- Buttons: add contentShape so the full pill is hit-testable, not just
  the label glyphs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rmarinsky rmarinsky added the release:patch Bug fix / internal — bumps patch version label May 15, 2026
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

🔖 On merge this PR will release v1.14.3 (release:patch).

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Button interactive areas are refined with clipped rounded rectangles. PermissionManager screen recording permission handling is changed to directly open System Settings. Onboarding permission request flows for microphone, accessibility, and screen recording are refactored to handle authorization status branching and explicit privacy pane navigation.

Changes

Permission Request and UI Polish

Layer / File(s) Summary
Button interactive area refinement
Diduny/Features/Onboarding/OnboardingContainerView.swift
OnboardingMainButton, PermissionActionButton, and PermissionOutlineButton each add a clipped contentShape(RoundedRectangle(...)) modifier to define interactive/hit-test boundaries.
PermissionManager screen recording direct settings access
Diduny/Core/Storage/PermissionManager.swift
ensureScreenRecordingPermission() removes the custom showPermissionAlert() flow and directly opens System Settings via openSystemSettingsForPermission() when screen recording is denied, aligning with macOS native first-request prompts.
Onboarding microphone, accessibility, and screen recording permission flows
Diduny/Features/Onboarding/OnboardingContainerView.swift
SetupComputerStepView.requestMicrophonePermission() refactors to switch on AVCaptureDevice.authorizationStatus(for: .audio) and opens microphone privacy settings when not authorized. requestAccessibilityPermission() explicitly opens accessibility privacy pane when access remains denied. ScreenRecordingStepView.requestScreenRecordingPermission() simplifies to call PermissionManager.shared.requestScreenRecordingPermission() directly and refresh via passive polling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • rmarinsky/Diduny#30: Both PRs modify the Screen Recording permission flow by changing/using PermissionManager helpers and updating ScreenRecordingStepView onboarding behavior around how screen-recording denial is handled.

Poem

🐰 Three buttons now know their shape,
Settings flow direct, no escape,
Permissions align with native ways,
Hit-tests shine bright, permission praise! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(onboarding): stop duplicate permission prompts (v1.14.3)' directly aligns with the core objective of eliminating duplicate permission dialogs in the onboarding flow. It clearly conveys the main change—removing duplicate prompts across microphone, accessibility, and screen recording.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch release/v1.14.3-permission-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rmarinsky rmarinsky merged commit 748755f into main May 15, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:patch Bug fix / internal — bumps patch version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant