feat(android): add photo-permission rationale and recent-photos strip#510
feat(android): add photo-permission rationale and recent-photos strip#510jkmassel wants to merge 1 commit into
Conversation
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/510")Built from 726a400 |
6353b11 to
0943b8a
Compare
764b1ab to
64a055f
Compare
Layers on the Photos / Camera tiles introduced in #509 with a full permission state machine, a rationale card, and a recent-photos thumbnail strip. - Three media-strip states resolved at runtime by `resolveMediaStripView` in `PhotoAccessState.kt`: 1. **Rationale card** — shown when `READ_MEDIA_IMAGES` isn't granted and the user hasn't dismissed it. Body copy and primary-button label switch across three sub-states (`Unasked` / `Denied` / `PermanentlyDenied`); SharedPreferences tracks the first-prompt flag because `shouldShowRequestPermissionRationale` alone can't tell "never asked" from "permanently denied". 2. **Compact tiles** — once the rationale is rejected, the Photos / Camera column flattens into a full-width 88dp row. The rejection auto-clears when permission is later granted, so a future revocation surfaces the rationale again. 3. **Full strip** — once granted, queries MediaStore for the 64 most recent images and renders them as a horizontally-scrolling 2-row thumbnail grid. LazyRow keeps off-screen thumbnails out of memory. - Strip is gated to Android 10+ (`ContentResolver.loadThumbnail` is API 29+); on Android 7-9 the inserter falls back to the permissionless Photos/Camera tile row and never requests the media permission. - Process-wide thumbnail cache keyed on (uri, sizePx) — 32 entries, ~6MB worst case. `RealThumbnail` seeds from the cache synchronously so scroll-back and dialog reopen skip the grey-placeholder flash. Failed-to-load URIs drop from the displayed strip and re-attempt on the next dialog open. - Android 14+ partial-grant treated as granted by also checking `READ_MEDIA_VISUAL_USER_SELECTED` when `READ_MEDIA_IMAGES` is denied. Without this, a "Select photos" choice fell through to the rationale's "Open Settings" state for a permission the user had just granted. - Photo-prefs reads off the main thread via a process-wide cache warmed from `GutenbergView`'s constructor on `Dispatchers.IO`. Writes update the cache synchronously and queue `SharedPreferences.edit().apply()` on the IO scope. - Soft-input mode `STATE_HIDDEN | ADJUST_RESIZE` — `STATE_HIDDEN` dismisses an in-flight IME on open; `ADJUST_RESIZE` lets the sheet shrink to make room when the user taps the in-dialog search field. - Observes the host Activity's lifecycle (not the BottomSheetDialog's own) for `ON_RESUME`, so grants made via system Settings update the strip on return without restart. - `GutenbergView.resetBlockPickerPhotoPreferences(context)` exposed for host apps that want to clear the rationale-rejection / first-prompt flags from a settings screen — also wired into the demo's `⋮` menu as **Manage Permissions**. Demo uses a Settings hand-off rather than `revokeSelfPermissionOnKill` (API 33+, demo's `minSdk = 24`). - Library declares `READ_MEDIA_IMAGES` and `READ_EXTERNAL_STORAGE` (max SDK 32). Host apps can opt out via `tools:node="remove"`; documented in `docs/integration.md` (Android → Manifest Permissions), including the `xmlns:tools` namespace requirement, with the opt-out XML inlined as a comment in `AndroidManifest.xml` for auditors diffing the merged manifest. - Demo's "Enable Native Inserter" toggle defaults to on so reviewers see the new strip without flipping a setting; the standalone-editor E2E test toggles it off so the existing web-inserter assertions still resolve. Touch targets on the rationale buttons meet the Material 48dp minimum via a wrapper that keeps the visual 32dp pill while inflating the click area; a shared `MutableInteractionSource` keeps the ripple drawing inside the pill instead of as a square halo. Verified on Pixel 9 Pro XL with \`./gradlew :Gutenberg:detekt :Gutenberg:assembleDebug :Gutenberg:testDebugUnitTest\` (includes \`PhotoAccessStateTest\`).
64a055f to
726a400
Compare
| access is PhotoAccess.Granted -> MediaStripView.FullStrip | ||
| rejected -> MediaStripView.CompactTiles | ||
| access is PhotoAccess.NeedsPermission -> MediaStripView.Rationale | ||
| else -> MediaStripView.FullStrip |
There was a problem hiding this comment.
Claude says this is an unreachable else. Probably because access has only two types.
In these cases, I always tend to avoid the else branch to force the developer to always make the when exhaustive in case new types are added.
adalpari
left a comment
There was a problem hiding this comment.
Some founds by Clause:
LaunchedEffect(access) in MediaStrip re-launches on every recomposition in NeedsPermission state (BlockPickerDialog.kt:273-278)
PhotoAccess.NeedsPermission contains a request: () -> Unit lambda that's freshly constructed on each call to rememberPhotoAccess. Data-class equality compares those lambdas by reference, so successive NeedsPermission values aren't equal even when their state is unchanged. That means the LaunchedEffect(access) { if (access is PhotoAccess.Granted && rejected) { ... } } cancels-and-relaunches on every recomposition during the rationale phase — wasteful, though benign because the inner body short-circuits.
Suggested fix: key the effect on access is PhotoAccess.Granted (a Boolean) instead of access, or pull granted out as a separate state in rememberPhotoAccess and key on that.LaunchedEffect(access) in MediaStrip re-launches on every recomposition in NeedsPermission state (BlockPickerDialog.kt:273-278)
PhotoAccess.NeedsPermission contains a request: () -> Unit lambda that's freshly constructed on each call to rememberPhotoAccess. Data-class equality compares those lambdas by reference, so successive NeedsPermission values aren't equal even when their state is unchanged. That means the LaunchedEffect(access) { if (access is PhotoAccess.Granted && rejected) { ... } } cancels-and-relaunches on every recomposition during the rationale phase — wasteful, though benign because the inner body short-circuits.
Suggested fix: key the effect on access is PhotoAccess.Granted (a Boolean) instead of access, or pull granted out as a separate state in rememberPhotoAccess and key on that.
Summary
Layers on top of #509 with the photo-permission state machine, the rationale card, and the recent-photos thumbnail strip.
Three media-strip states resolved at runtime by
resolveMediaStripViewinPhotoAccessState.kt:READ_MEDIA_IMAGESisn't granted and the user hasn't dismissed it. Body copy and primary-button label swap across three sub-states (Unasked/Denied/PermanentlyDenied); SharedPreferences tracks the first-prompt flag becauseshouldShowRequestPermissionRationalealone can't tell "never asked" from "permanently denied".clearRejectedRationale, so a future revocation surfaces the rationale again instead of stranding the user in CompactTiles with no in-app affordance to re-engage.LazyRowkeeps off-screen thumbnails out of memory.Sorry, this one is a little bigger, but I couldn't find a meaningful way to split it up.
Lifecycle observer
The strip observes the host Activity's lifecycle (via
LocalContext.findActivity()walking theContextWrapperchain), not theBottomSheetDialog's ownLifecycleRegistry. The dialog's lifecycle only dispatchesON_RESUMEfromshow()and never refires when the user leaves to system Settings and returns; the Activity's does. This is what makes the "grant via Settings → strip updates on return" path work even when the inserter sheet stays open through the round-trip.canRepromptstateModelled as its own
mutableStateOf, explicitly recomputed at every permission-relevant signal (launcher result, lifecycle resume) via therefreshAccessStatehelper. Compose's snapshot system can't see the OS-levelshouldShowRequestPermissionRationaleflip fromtrue → falseon the 2nd denial otherwise —grantedandpromptedBeforeare already in their post-deny values, so neither would notify Compose and the rationale would stay stuck on "Try Again".Manifest permissions
This PR adds two host-visible
<uses-permission>declarations to the library manifest:READ_MEDIA_IMAGESREAD_EXTERNAL_STORAGEAndroid 14+'s three-option permission dialog ("Allow all" / "Select photos" / "Don't allow") is surfaced by the system whenever an app at targetSdk ≥ 34 requests
READ_MEDIA_IMAGES, regardless of whether the manifest opts in toREAD_MEDIA_VISUAL_USER_SELECTED(verified empirically on Pixel 9 Pro XL / Android 16).hasPhotosPermissionchecks both permissions, so a "Select photos" choice is treated as granted and MediaStore scopes the strip to the user-selected items. DeclaringREAD_MEDIA_VISUAL_USER_SELECTEDin the library manifest to unlock the "Select more photos" re-prompt UX is #511.Hosts that don't ship the media strip can opt out:
With the permissions removed,
hasPhotosPermissionreturnsfalse, the rationale is unreachable (the launcher request hits the merged-manifest check and resolves to denied), and the strip falls back to the CompactTiles layout from the parent PR. The Photos tile (system picker) and Camera tile continue to work — they're permissionless.Host integration
GutenbergView.resetBlockPickerPhotoPreferences(context)is exposed for host apps that want to clear the rationale-rejection / first-prompt flags from a settings screen.The demo's
⋮menu now has a Manage Permissions screen for replaying the rationale flow during review: it shows the current OS state forREAD_MEDIA_IMAGES, lets reviewers reset the in-app flags via the library API above, and hands off to the system permission settings for OS-level revokes.revokeSelfPermissionOnKillwas the obvious path for the OS-revoke side but requires API 33+; the demo'sminSdk = 24, so a Settings hand-off code path is needed regardless. Settings also gives the user an OS-owned confirmation surface and avoids the lifecycle race of an async self-kill. The screen refreshes onON_RESUMEso returning from Settings reflects the new state immediately.The demo's "Enable Native Inserter" toggle now defaults to on so reviewers see the new strip without flipping a setting.
Test plan
⋮→ Manage Permissions shows current OS state (Granted/Denied (system will re-prompt)/Denied (system prompt suppressed)).ON_RESUMEobserver../gradlew :Gutenberg:detekt :Gutenberg:assembleDebug :Gutenberg:testDebugUnitTestpasses (includesPhotoAccessStateTest).