Skip to content

feat(android): add photo-permission rationale and recent-photos strip#510

Draft
jkmassel wants to merge 4 commits into
trunkfrom
jkmassel/block-picker-photo-permissions
Draft

feat(android): add photo-permission rationale and recent-photos strip#510
jkmassel wants to merge 4 commits into
trunkfrom
jkmassel/block-picker-photo-permissions

Conversation

@jkmassel
Copy link
Copy Markdown
Contributor

@jkmassel jkmassel commented May 13, 2026

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 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 swap 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 via clearRejectedRationale, so a future revocation surfaces the rationale again instead of stranding the user in CompactTiles with no in-app affordance to re-engage.
  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.

Lifecycle observer

The strip observes the host Activity's lifecycle (via LocalContext.findActivity() walking the ContextWrapper chain), not the BottomSheetDialog's own LifecycleRegistry. The dialog's lifecycle only dispatches ON_RESUME from show() 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.

canReprompt state

Modelled as its own mutableStateOf, explicitly recomputed at every permission-relevant signal (launcher result, lifecycle resume) via the refreshAccessState helper. Compose's snapshot system can't see the OS-level shouldShowRequestPermissionRationale flip from true → false on the 2nd denial otherwise — granted and promptedBefore are 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:

Permission SDK range Purpose
READ_MEDIA_IMAGES 33+ Read MediaStore images for the thumbnail strip.
READ_EXTERNAL_STORAGE ≤32 Pre-Android-13 fallback for the same read.

Android 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 to READ_MEDIA_VISUAL_USER_SELECTED (verified empirically on Pixel 9 Pro XL / Android 16). hasPhotosPermission checks both permissions, so a "Select photos" choice is treated as granted and MediaStore scopes the strip to the user-selected items. Declaring READ_MEDIA_VISUAL_USER_SELECTED in 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:

<uses-permission
    android:name="android.permission.READ_MEDIA_IMAGES"
    tools:node="remove" />
<uses-permission
    android:name="android.permission.READ_EXTERNAL_STORAGE"
    tools:node="remove" />

With the permissions removed, hasPhotosPermission returns false, 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 for READ_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. revokeSelfPermissionOnKill was the obvious path for the OS-revoke side but requires API 33+; the demo's minSdk = 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 on ON_RESUME so 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

Note: the Photos and Camera tiles open the system picker / camera app, but the picked URI / capture is not yet handed off to the JS editor for block insertion — that round-trip lands in a follow-up PR. The launchers firing and the permission/rationale flow are the parts under test here.

  • Open the inserter on first launch. Rationale card shows Allow + Reject alongside the Photos/Camera column.
  • Tap Allow → system prompt → grant → strip flips to recent photos, behind the same Photos/Camera column.
  • Deny system prompt once → rationale switches to Try Again.
  • Deny system prompt twice → rationale switches to Open Settings (system suppresses the prompt).
  • Tap Reject in rationale → rationale hides, Photos/Camera reflow into a 88dp full-width row; preference persists across dialog dismiss/relaunch and app cold-start.
  • After Reject, grant permission via system Settings → return to the app → strip flips to thumbnails (lifecycle resume picks up the new grant).
  • After grant, revoke via system Settings → return to the app → rationale card returns (sticky rejected flag was cleared on grant).
  • Tap Photos tile → system photo picker opens (works in all three states).
  • Tap Camera tile → camera app opens with a cache-scoped output URI (works in all three states).
  • Vertical drag on the photo strip still drags the bottom sheet (LazyRow's nested-scroll dispatch).
  • Demo Manage Permissions shows current OS state (Granted / Denied (system will re-prompt) / Denied (system prompt suppressed)).
  • Reset flags clears the SharedPreferences; reopening the inserter shows the rationale fresh.
  • Open Settings hands off to the system permission detail; toggling there and returning refreshes the state card via the ON_RESUME observer.
  • Rationale buttons feel comfortably tappable despite their 32dp visual.
  • ./gradlew :Gutenberg:detekt :Gutenberg:assembleDebug :Gutenberg:testDebugUnitTest passes (includes PhotoAccessStateTest).

@wpmobilebot
Copy link
Copy Markdown

wpmobilebot commented May 13, 2026

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/510")

Built from 764b1ab

@jkmassel jkmassel force-pushed the jkmassel/block-picker-photos-camera branch 6 times, most recently from 6353b11 to 0943b8a Compare May 13, 2026 18:40
@jkmassel jkmassel marked this pull request as draft May 13, 2026 19:14
Base automatically changed from jkmassel/block-picker-photos-camera to trunk May 14, 2026 15:08
@jkmassel jkmassel force-pushed the jkmassel/block-picker-photo-permissions branch 4 times, most recently from 04fa2fc to d7bd4ad Compare May 14, 2026 23:22
jkmassel added 4 commits May 15, 2026 11:49
Layers on the Photos / Camera tiles introduced in the previous PR 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.
- 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**.
- 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),
  with the opt-out XML inlined as a comment in `AndroidManifest.xml` for
  auditors diffing the merged manifest.
- Demo's "Enable Native Inserter" toggle now defaults to on so
  reviewers see the new strip without flipping a setting.

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.
…-ups

Iterative follow-ups to the inserter's photo-permission strip after
empirical testing on Pixel 9 Pro XL / Android 16:

- **Process-wide thumbnail cache** keyed on `(uri, sizePx)` to skip the
  binder + GC churn of `ContentResolver.loadThumbnail` on scroll-back
  and dialog reopen. `RealThumbnail` seeds from the cache synchronously
  at composition so cache hits skip the grey-placeholder flash entirely.
  Sized at 32 entries (~6MB worst case at 64dp/3.5x density). Including
  `sizePx` in the key means a density change can't serve a wrong-sized
  bitmap.
- **Drop unloadable thumbnails** from the displayed strip rather than
  rendering a permanent grey placeholder. Failed URIs are tracked in a
  `failed: Set<Uri>` keyed on the `Granted` instance and re-attempted
  on the next dialog open. `Log.w` on failure for engineer visibility
  (matches `EditorHTTPClient` / `EditorAssetsLibrary` convention).
- **Soft-input mode** changed from `STATE_HIDDEN | ADJUST_NOTHING` to
  `STATE_HIDDEN | ADJUST_RESIZE`. The latched `ADJUST_NOTHING` was
  letting the IME cover the search field on in-dialog search taps;
  `STATE_HIDDEN` alone fixes the original double-launch race.
- **Android 14 partial-grant** treated as granted by also checking
  `READ_MEDIA_VISUAL_USER_SELECTED` (API 34+) 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. MediaStore automatically scopes the recent-images query to
  the user-selected items.
- **Photo-prefs reads off the main thread** via a process-wide
  `PhotoPrefs` cache populated by `Dispatchers.IO` warmup dispatched
  from `GutenbergView`'s constructor. Writes update the cache
  synchronously (so subsequent reads see the new value) and queue the
  `SharedPreferences.edit().apply()` on the IO scope. Synchronous prefs
  access during the sheet's open animation was a StrictMode
  `detectDiskReads()` violation.

Verified on Pixel 9 Pro XL with `./gradlew :Gutenberg:detekt
:Gutenberg:assembleDebug :Gutenberg:testDebugUnitTest`.
The previous comment claimed Android 16 "doesn't reliably honor"
`revokeSelfPermissionOnKill` based on empirical observation that the
process died but the grant stuck. That's consistent with the docs'
"may delay the revocation" language — the API schedules a revocation
for the next *safe* kill, which isn't necessarily the kill the caller
observes — so the platform-blaming overclaims.

The real reason to route through system Settings is simpler and harder
to argue with: `revokeSelfPermissionOnKill` is API 33+ only, and the
demo's `minSdk = 24`. We'd need a Settings hand-off code path
regardless. Settings also gives the user an OS-owned confirmation
surface and avoids the lifecycle race of an async self-kill.

Comment-only change in the demo app; no behaviour change.
The standalone-editor E2E test (`EditorTestHelpers.insertBlock`) taps
"Add block" in the editor toolbar and waits for the web Gutenberg
inserter (`[role='dialog'][aria-modal='true']`). With the demo's
`enableNativeInserter` toggle defaulted to on (this PR), the same tap
opens the native `BlockPickerDialog` bottom sheet instead, so the
WebView selector never resolves and the test times out.

Two narrow changes:

- Add a `contentDescription = "Enable Native Inserter"` semantics
  modifier to the Switch in `SitePreparationActivity` so the test can
  find it without coupling to switch-order or sibling-text traversal.
- In `EditorTestHelpers.navigateToEditor`, toggle the switch off after
  the configuration screen renders and before tapping Start. The
  WebView-side test path then drives the web inserter as before.

The default-on toggle is preserved for reviewer ergonomics — fresh
demo installs still see the photo-permission strip on first inserter
open. The native inserter has its own manual test plan in this PR's
description; the existing E2E suite continues to cover the web flow.
@jkmassel jkmassel force-pushed the jkmassel/block-picker-photo-permissions branch from 764b1ab to 64a055f Compare May 15, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants