Add media library permissions handling and photo access components #352
Conversation
The commit improves user experience by implementing better loading states and empty state handling, including a new LoadingEventItem component and refined conditional rendering logic for various feed states.
The commit improves the photo library access experience by: - Adding support for partial photo access on iOS - Introducing a dedicated PhotoAccessPrompt component - Implementing ActionSheet for managing photo permissions
The commit improves media library access by adding support for partial photo access, implementing proper permission checks, and optimizing photo grid display based on permission status.
The commit reorganizes the media library handling in the NewEventModal component, introducing better separation of concerns and more reliable permission checks through useFocusEffect.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThe pull request focuses on enhancing media permissions and photo access functionality across multiple components in an Expo application. The changes introduce a comprehensive approach to managing media library access, including checking permissions, handling user interactions, and providing clear UI guidance. The modifications span several files, adding new components like Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant MediaLibrary
participant DeviceSettings
User->>App: Attempts to access photos
App->>MediaLibrary: Check Permissions
alt No Media Permission
App->>User: Show PhotoAccessPrompt
User->>DeviceSettings: Enable Photo Access
DeviceSettings-->>MediaLibrary: Update Permissions
end
MediaLibrary-->>App: Confirm Permissions
App->>App: Load Recent Photos
App->>User: Display Photo Grid
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
apps/expo/src/hooks/useMediaPermissions.ts (2)
12-21: Handle permission statuses more granularly if required.
Right now, the code treats both partial and fully granted permissions uniformly forisGranted. If you need special logic for limited access (e.g., “selected photos” on iOS), you might separate checks for each permission type (e.g., “limited,” “denied,” etc.).
39-39: Confirm necessity of bothsetHasMediaPermissionandsetRecentPhotos.
Although you’re referencing both store setters in the dependency array,setHasMediaPermissionisn’t directly used in this effect definition. Verify whether you need it here to avoid unnecessary re-renders.apps/expo/src/app/new.tsx (6)
64-68: Props are well-structured but watch for future complexity.
ExpandingPhotoGridwith extra permission flags is fine for now. If you continue adding more permission types, consider an all-in-one permission object.
85-89: Don’t pass empty array if permission is denied.
This approach to conditionally setdataimproves privacy by hiding photos from the grid. If you need to prompt the user more strongly for permission, consider an alternative UI that directs them to “request permission.”
107-120: Support partial library permission states contextually.
The user sees the “Manage” button only if they have partial library access. This is a good pattern. Double-check the copy for clarity, especially if you later add more granular states (e.g., “limited”).
539-544: Offline or ephemeral states might need error handling.
Currently, you rely on re-focusing to reload. If the user is offline, consider storing partial state or gracefully handling errors insetPhotoLoadingError.
576-576: Console logging the most recent photo is fine.
If this is only for debugging, remove or wrap it with a debug flag to avoid noise for production.
720-816: Preview container logic is large; consider splitting.
The multiple conditions for either image preview, link preview, or text input can become unwieldy. Factor out a smaller component if this grows further.apps/expo/src/components/PhotoAccessPrompt.tsx (1)
10-16: Good practice: direct user to system settings.
This approach is typical for partial media permissions. Consider including a direct “requestPermissionsAsync” fallback for better control on Android.apps/expo/src/store.ts (1)
221-222: Check usage of isLoadingPhotos / photoLoadingError.
These new states are settable but aren’t used extensively. Consider a loading spinner or error message in the UI to improve the user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/expo/src/app/new.tsx(9 hunks)apps/expo/src/components/AddEventButton.tsx(1 hunks)apps/expo/src/components/PhotoAccessPrompt.tsx(1 hunks)apps/expo/src/hooks/useMediaPermissions.ts(1 hunks)apps/expo/src/store.ts(3 hunks)
🔇 Additional comments (21)
apps/expo/src/hooks/useMediaPermissions.ts (3)
7-7: Use destructured setters for clarity.
Using destructured store setters is effective for readability. Just ensure you invoke them consistently within your codebase to reduce ambiguity.
23-28: Validate potential repeated subscriptions.
WhenisGrantedis true,checkPermissionsAndSubscribere-subscribes after incremental changes, which could result in multiple active subscriptions if frequent changes occur. Although you clear the subscription on unmount, consider ensuring you don’t stack multiple subscriptions.
32-32: Immediate invocation is well-organized.
CallingcheckPermissionsAndSubscribe()upon mount is a good approach to ensure immediate permission checks and subscriptions.apps/expo/src/app/new.tsx (11)
3-3: Platform-specific consideration for ActionSheetIOS.
ImportingActionSheetIOSexplicitly ties the code to iOS. Ensure the app gracefully handles or bypasses this flow on Android.
37-37: Recheck necessity of PhotoAccessPrompt import.
The import seems essential, but ensure there’s no circular dependency or leftover references if you later relocate the prompt.
81-83: Inline handler is straightforward.
handleManagePresssimply opens settings, which is consistent with iOS partial-permissions workflows.
91-96: Clear heading text.
This “Recents” heading is helpful. Just ensure it’s localized or consistent with your design guidelines if needed.
124-124: Appropriate key assignment for FlatList data.
Usingidis suitable. If new placeholders or logic are added, ensure uniqueidgeneration.
226-226: Ensure consistent usage of hasFullPhotoAccess.
You’ve destructuredhasFullPhotoAccess; confirm it’s used consistently throughout.
462-535: In-depth focus effect is well-organized but watch for repeated permission calls.
You’re loading permissions and photos each focus. That’s good to stay fresh, but ensure a user’s repeated navigations don’t cause excessive calls. Possibly skip ifhasMediaPermissionis already up to date.
717-719: Conditional usage of PhotoAccessPrompt.
Nicely done to handle prompting only if permission is absent and not from an external intent. Verify that “describe” is always a safe override.
819-827: PhotoGrid usage is consistent.
PassinghasMediaPermissionandhasFullPhotoAccesskeeps the logic in the same domain. Good job.
833-848: Create event button approach is user-friendly.
Disabling the button if no input is present avoids spurious requests. TheCapture eventlabel is clear.
850-850: Overall structural clarity.
The final bracket ends a large conditional block. Everything looks consistent.apps/expo/src/components/PhotoAccessPrompt.tsx (3)
1-2: Confirm that Linking to Settings is cross-platform.
On iOS,openSettings()function typically routes to the app settings. On Android, this may open a default settings screen or sometimes do nothing. Consider a fallback approach for other platforms.
5-8: Prop naming clarity is good.
onCloseandshowCloseButtonare intuitive. Keep them consistent if you add more prompt variants.
18-47: UI layout is minimal and direct.
Centering the message is user friendly. The text clearly indicates why the permission is needed. Nicely done.apps/expo/src/store.ts (2)
72-72: Adding a separate hasFullPhotoAccess flag is beneficial
Separating partial from full permission states is helpful for handling iOS “limited” access. Good addition.
204-204: Keep partial permission states in sync.
You set defaulthasFullPhotoAccess: false. Ensure that any manual permission updates or partial-limited toggles do not incorrectly override.apps/expo/src/components/AddEventButton.tsx (2)
13-13: Decoupled permission logic.
RetrievinghasMediaPermissiondirectly from the store is clean. Just ensure you handle partial permission states if you plan to differentiate “limited” vs “full.”
16-20: Conditional logic is straightforward.
Early return if permission is already present is a good design. Minimizes redundant permission requests.
| onPress={() => { | ||
| if (hasMediaPermission && !hasFullPhotoAccess) { | ||
| // Show action sheet for partial access | ||
| ActionSheetIOS.showActionSheetWithOptions( | ||
| { | ||
| options: [ | ||
| "Select More Photos", | ||
| "Change Settings", | ||
| "Cancel", | ||
| ], | ||
| cancelButtonIndex: 2, | ||
| }, | ||
| (buttonIndex) => { | ||
| if (buttonIndex === 0) { | ||
| void MediaLibrary.presentPermissionsPickerAsync(); | ||
| } else if (buttonIndex === 1) { | ||
| void Linking.openSettings(); | ||
| } | ||
| }, | ||
| ); | ||
| } else { | ||
| onMorePhotos(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ActionSheet iOS block might need Android fallback.
Relying on ActionSheetIOS.showActionSheetWithOptions can disrupt cross-platform usage. Ensure you degrade gracefully on Android, or wrap it with a suitable platform check.
There was a problem hiding this comment.
not focusing on ios for now
There was a problem hiding this comment.
@jaronheard Understood! Since you’re not focusing on cross-platform functionality at the moment, we can skip adding a fallback on Android for now. Let me know if you want to revisit this in the future.
| router.push("/new"); | ||
| } catch (error) { | ||
| console.error("Error requesting media permissions:", error); | ||
| router.push("/new"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider not navigating if user denies.
Even if an error or denial occurs, you still call router.push("/new"). That might confuse users who can’t actually pick images. Possibly remain on the current screen or show a prompt if the user denies.
There was a problem hiding this comment.
@jaronheard Understood—thanks for clarifying. We’ll keep the existing navigation flow as is. Let me know if anything else comes up!
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
The update focuses on improving photo access and permissions, providing users with clearer guidance and more control over their media library interactions.