Skip to content

Commit a086223

Browse files
committed
fix(android): address review feedback on block inserter media strip
- Observe the host Activity's lifecycle (not the BottomSheetDialog's) so the photo-access state refreshes when the user returns from system Settings. The dialog's own LifecycleRegistry only dispatches ON_RESUME from `show()` and never refires on activity resume. - Switch the recent-photo strip to LazyRow so the 64 thumbnails aren't all decoded into memory upfront. - Replace the `permissionTick` snapshot-read trick with a `canReprompt` state, written explicitly by the launcher callback and resume observer. - Declare `READ_MEDIA_VISUAL_USER_SELECTED`, request both photo permissions together via `RequestMultiplePermissions`, detect partial grants, and surface a "Manage" tile in the strip when only partial is granted (Android 14+). - Clear the rationale-rejected flag once the permission is observed granted, so a later revocation surfaces the rationale again instead of trapping the user in the compact-tiles state. - Inflate touch targets on rationale buttons and category chips to the Material 48dp minimum without changing the visual heights, using a shared MutableInteractionSource so the ripple still draws inside the rounded pill. - Drop the deprecated `androidx.compose.ui.platform.LocalLifecycleOwner` import. - Add a TODO for cleaning up orphaned camera capture files when the editor URI hand-off lands. - Default the demo's "Enable Native Inserter" toggle to on so reviewers see the new sheet without flipping a setting.
1 parent cf866f4 commit a086223

7 files changed

Lines changed: 277 additions & 93 deletions

File tree

android/Gutenberg/src/main/AndroidManifest.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
<!-- Used by the block inserter media strip to preview recent device photos.
66
Host apps that don't need this can opt out via tools:node="remove". -->
77
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
8+
<!-- Android 14+ partial-access opt-in. Declaring this lets the system offer
9+
"Select photos and videos" alongside the all/none choice and lets us
10+
detect partial grants so we can surface a "Manage selection" affordance. -->
11+
<uses-permission android:name="android.permission.READ_MEDIA_VISUAL_USER_SELECTED" />
812
<uses-permission
913
android:name="android.permission.READ_EXTERNAL_STORAGE"
1014
android:maxSdkVersion="32" />

android/Gutenberg/src/main/java/org/wordpress/gutenberg/inserter/BlockPickerDialog.kt

Lines changed: 144 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import androidx.compose.foundation.Image
1313
import androidx.compose.foundation.background
1414
import androidx.compose.foundation.border
1515
import androidx.compose.foundation.clickable
16+
import androidx.compose.foundation.indication
17+
import androidx.compose.foundation.interaction.MutableInteractionSource
1618
import androidx.compose.foundation.gestures.Orientation
1719
import androidx.compose.foundation.gestures.rememberScrollableState
1820
import androidx.compose.foundation.gestures.scrollable
@@ -32,6 +34,7 @@ import androidx.compose.foundation.layout.heightIn
3234
import androidx.compose.foundation.layout.padding
3335
import androidx.compose.foundation.layout.size
3436
import androidx.compose.foundation.layout.width
37+
import androidx.compose.foundation.lazy.LazyRow
3538
import androidx.compose.foundation.lazy.grid.GridCells
3639
import androidx.compose.foundation.lazy.grid.LazyVerticalGrid
3740
import androidx.compose.foundation.rememberScrollState
@@ -42,11 +45,13 @@ import androidx.compose.material.icons.filled.Close
4245
import androidx.compose.material.icons.filled.PhotoCamera
4346
import androidx.compose.material.icons.filled.PhotoLibrary
4447
import androidx.compose.material.icons.filled.Search
48+
import androidx.compose.material.icons.filled.Tune
4549
import androidx.compose.material3.ColorScheme
4650
import androidx.compose.material3.Icon
4751
import androidx.compose.material3.IconButton
4852
import androidx.compose.material3.MaterialTheme
4953
import androidx.compose.material3.Text
54+
import androidx.compose.material3.ripple
5055
import androidx.compose.material3.darkColorScheme
5156
import androidx.compose.material3.dynamicDarkColorScheme
5257
import androidx.compose.material3.dynamicLightColorScheme
@@ -197,6 +202,13 @@ private const val EMPTY_STATE_FONT_SP = 14
197202

198203
private const val DISABLED_ALPHA = 0.5f
199204

205+
/**
206+
* Material 3 minimum touch-target. Several visual elements here (rationale
207+
* buttons, category chips) sit below this for design reasons; we wrap them in
208+
* an outer clickable that meets the minimum without changing the visual size.
209+
*/
210+
private const val TOUCH_TARGET_MIN_DP = 48
211+
200212
/**
201213
* Bottom-sheet block inserter. The outer shell stays a `BottomSheetDialog` so
202214
* `GutenbergView`'s integration surface is unchanged; everything visible is
@@ -454,6 +466,15 @@ private fun MediaStrip() {
454466
val access = rememberPhotoAccess(limit = RECENT_PHOTO_LIMIT)
455467
val context = LocalContext.current
456468
var rejected by remember { mutableStateOf(hasRejectedRationale(context)) }
469+
// Clear a prior rejection once the user actually grants the permission.
470+
// Without this, a later revocation would leave the user in CompactTiles
471+
// with no in-app path back to the rationale.
472+
LaunchedEffect(access) {
473+
if (access is PhotoAccess.Granted && rejected) {
474+
clearRejectedRationale(context)
475+
rejected = false
476+
}
477+
}
457478
val contentPadding = PaddingValues(
458479
start = MEDIA_STRIP_CONTENT_HORIZONTAL_PAD_DP.dp,
459480
end = MEDIA_STRIP_CONTENT_HORIZONTAL_PAD_DP.dp,
@@ -496,28 +517,64 @@ private fun MediaStrip() {
496517
modifier = Modifier.fillMaxWidth().padding(contentPadding),
497518
)
498519
}
499-
MediaStripView.FullStrip -> {
500-
// Granted — thumbnail grid extends past the viewport, so use
501-
// horizontalScroll + a zero-consuming vertical relay so vertical
502-
// drags still reach BottomSheetBehavior.
503-
val scrollState = rememberScrollState()
504-
val verticalRelay = rememberScrollableState { 0f }
505-
val uris = (access as? PhotoAccess.Granted)?.uris.orEmpty()
506-
Row(
507-
horizontalArrangement = Arrangement.spacedBy(MEDIA_STRIP_ITEM_GAP_DP.dp),
508-
modifier = Modifier
509-
.horizontalScroll(scrollState)
510-
.scrollable(verticalRelay, Orientation.Vertical)
511-
.padding(contentPadding),
512-
) {
513-
PhotosCameraTile(horizontal = false)
514-
if (uris.isNotEmpty()) MediaThumbnailGrid(uris = uris)
520+
MediaStripView.FullStrip -> FullMediaStrip(
521+
granted = access as? PhotoAccess.Granted,
522+
contentPadding = contentPadding,
523+
)
524+
}
525+
}
526+
}
527+
528+
@Composable
529+
private fun FullMediaStrip(granted: PhotoAccess.Granted?, contentPadding: PaddingValues) {
530+
// Granted — thumbnail strip extends past the viewport. LazyRow composes
531+
// only the visible columns (plus prefetch), so the 64 thumbnails aren't
532+
// all decoded into memory upfront. Vertical drags pass through the lazy
533+
// list's nested-scroll dispatch up to BottomSheetBehavior; no manual
534+
// relay needed.
535+
val uris = granted?.uris.orEmpty()
536+
val partialAccess = granted?.partialAccess
537+
val columns = (uris.size + 1) / 2
538+
LazyRow(
539+
horizontalArrangement = Arrangement.spacedBy(MEDIA_STRIP_ITEM_GAP_DP.dp),
540+
contentPadding = contentPadding,
541+
modifier = Modifier.fillMaxWidth(),
542+
) {
543+
item(key = "actions") { PhotosCameraTile(horizontal = false) }
544+
if (partialAccess != null) {
545+
// Sits right after Photos/Camera so the affordance is visible without
546+
// scrolling — partial-access users won't otherwise have any in-app
547+
// path to update their selection.
548+
item(key = "manage") {
549+
ManageSelectionTile(onClick = partialAccess.onManageSelection)
550+
}
551+
}
552+
items(columns, key = { uris[it * 2].toString() }) { col ->
553+
Column(verticalArrangement = Arrangement.spacedBy(MEDIA_STRIP_ITEM_GAP_DP.dp)) {
554+
RealThumbnail(uri = uris[col * 2])
555+
val secondIndex = col * 2 + 1
556+
if (secondIndex < uris.size) {
557+
RealThumbnail(uri = uris[secondIndex])
515558
}
516559
}
517560
}
518561
}
519562
}
520563

564+
@Composable
565+
private fun ManageSelectionTile(onClick: () -> Unit) {
566+
MediaActionTile(
567+
iconVector = Icons.Filled.Tune,
568+
label = stringResource(R.string.gbk_block_inserter_photos_manage),
569+
background = MaterialTheme.colorScheme.secondaryContainer,
570+
foreground = MaterialTheme.colorScheme.onSecondaryContainer,
571+
onClick = onClick,
572+
modifier = Modifier
573+
.width(MEDIA_STACK_WIDTH_DP.dp)
574+
.height(MEDIA_STACK_HEIGHT_DP.dp),
575+
)
576+
}
577+
521578
@Composable
522579
@Suppress("LongMethod")
523580
private fun PhotosCameraTile(
@@ -527,6 +584,12 @@ private fun PhotosCameraTile(
527584
val context = LocalContext.current
528585
// System photo picker is permissionless — our READ_MEDIA_IMAGES permission
529586
// only gates the recent-photos strip, not this launch path.
587+
//
588+
// The result callbacks below are intentionally inert: the picked URI / camera
589+
// capture needs to round-trip through `WebViewAssetLoader` so the JS editor
590+
// can `fetch()` it, which is a follow-up. Until that lands, this whole sheet
591+
// is gated behind the demo app's "Enable Native Inserter" toggle, so users
592+
// outside that opt-in won't see the no-op buttons.
530593
val photoPicker = rememberLauncherForActivityResult(
531594
ActivityResultContracts.PickVisualMedia()
532595
) { /* picked uri — hand-off to editor insertion is a follow-up */ }
@@ -601,6 +664,10 @@ private fun PhotosCameraTile(
601664
private fun createCameraOutputUri(context: Context): Uri {
602665
val dir = File(context.cacheDir, "camera").apply { mkdirs() }
603666
val file = File(dir, "capture_${System.currentTimeMillis()}.jpg")
667+
// TODO: clean up captured files once the editor hand-off lands. Each Camera
668+
// tap creates a fresh file here; with the result callback inert today, every
669+
// capture is orphaned in the cache. When we wire up the URI hand-off, delete
670+
// on success/cancel and sweep stale files on next entry.
604671
return FileProvider.getUriForFile(
605672
context,
606673
"${context.packageName}.gutenberg.fileprovider",
@@ -640,25 +707,6 @@ private fun MediaActionTile(
640707
}
641708
}
642709

643-
@Composable
644-
private fun MediaThumbnailGrid(uris: List<Uri>) {
645-
// Two rows of tiles laid out left-to-right, column-by-column. Only render
646-
// as many tiles as we have URIs — empty slots otherwise would render
647-
// colorful mock placeholders that flash on the way to the real bitmaps.
648-
val columns = (uris.size + 1) / 2
649-
Row(horizontalArrangement = Arrangement.spacedBy(MEDIA_STRIP_ITEM_GAP_DP.dp)) {
650-
repeat(columns) { col ->
651-
Column(verticalArrangement = Arrangement.spacedBy(MEDIA_STRIP_ITEM_GAP_DP.dp)) {
652-
RealThumbnail(uri = uris[col * 2])
653-
val secondIndex = col * 2 + 1
654-
if (secondIndex < uris.size) {
655-
RealThumbnail(uri = uris[secondIndex])
656-
}
657-
}
658-
}
659-
}
660-
}
661-
662710
@Composable
663711
private fun PhotoAccessRationale(
664712
state: PromptState,
@@ -680,20 +728,38 @@ private fun PhotoAccessRationale(
680728
fun RationaleButton(label: String, filled: Boolean, onClick: () -> Unit, modifier: Modifier) {
681729
val bg = if (filled) MaterialTheme.colorScheme.primary else ComposeColor.Transparent
682730
val fg = if (filled) MaterialTheme.colorScheme.onPrimary else MaterialTheme.colorScheme.primary
731+
// Outer wrapper inflates the touch zone to the 48dp minimum; the inner
732+
// Box paints the design's 32dp pill. The shared interaction source lets
733+
// the outer absorb taps with no indication while the inner draws the
734+
// ripple — otherwise a default ripple on the outer would render as a
735+
// square halo around the rounded pill.
736+
val interactionSource = remember { MutableInteractionSource() }
683737
Box(
684738
contentAlignment = Alignment.Center,
685739
modifier = modifier
686-
.height(MEDIA_RATIONALE_BUTTON_HEIGHT_DP.dp)
687-
.clip(RoundedCornerShape(MEDIA_RATIONALE_BUTTON_CORNER_DP.dp))
688-
.background(bg)
689-
.clickable(onClick = onClick),
740+
.heightIn(min = TOUCH_TARGET_MIN_DP.dp)
741+
.clickable(
742+
interactionSource = interactionSource,
743+
indication = null,
744+
onClick = onClick,
745+
),
690746
) {
691-
Text(
692-
text = label,
693-
color = fg,
694-
fontSize = MEDIA_RATIONALE_BUTTON_FONT_SP.sp,
695-
fontWeight = FontWeight.Medium,
696-
)
747+
Box(
748+
contentAlignment = Alignment.Center,
749+
modifier = Modifier
750+
.fillMaxWidth()
751+
.height(MEDIA_RATIONALE_BUTTON_HEIGHT_DP.dp)
752+
.clip(RoundedCornerShape(MEDIA_RATIONALE_BUTTON_CORNER_DP.dp))
753+
.background(bg)
754+
.indication(interactionSource, ripple()),
755+
) {
756+
Text(
757+
text = label,
758+
color = fg,
759+
fontSize = MEDIA_RATIONALE_BUTTON_FONT_SP.sp,
760+
fontWeight = FontWeight.Medium,
761+
)
762+
}
697763
}
698764
}
699765
Column(
@@ -816,27 +882,43 @@ private fun CategoryChip(
816882
} else {
817883
MaterialTheme.colorScheme.outlineVariant
818884
}
885+
// Outer wrapper takes the click and the 48dp minimum-touch height; the
886+
// inner Box keeps the design's 36dp pill with border + background. Shared
887+
// interaction source so the ripple draws inside the rounded pill instead
888+
// of as a square over the wrapper's rectangular bounds.
889+
val interactionSource = remember { MutableInteractionSource() }
819890
Box(
820891
contentAlignment = Alignment.Center,
821892
modifier = Modifier
822-
.height(CHIP_HEIGHT_DP.dp)
823-
.clip(RoundedCornerShape(CHIP_CORNER_DP.dp))
824-
.background(background)
825-
.border(
826-
width = CHIP_BORDER_WIDTH_DP.dp,
827-
color = borderColor,
828-
shape = RoundedCornerShape(CHIP_CORNER_DP.dp),
829-
)
830-
.clickable(onClick = onClick)
831-
.padding(horizontal = CHIP_HORIZONTAL_PAD_DP.dp),
893+
.heightIn(min = TOUCH_TARGET_MIN_DP.dp)
894+
.clickable(
895+
interactionSource = interactionSource,
896+
indication = null,
897+
onClick = onClick,
898+
),
832899
) {
833-
Text(
834-
text = label,
835-
color = textColor,
836-
fontSize = CHIP_FONT_SP.sp,
837-
fontWeight = FontWeight.Medium,
838-
letterSpacing = CHIP_LETTER_SPACING_SP.sp,
839-
)
900+
Box(
901+
contentAlignment = Alignment.Center,
902+
modifier = Modifier
903+
.height(CHIP_HEIGHT_DP.dp)
904+
.clip(RoundedCornerShape(CHIP_CORNER_DP.dp))
905+
.background(background)
906+
.border(
907+
width = CHIP_BORDER_WIDTH_DP.dp,
908+
color = borderColor,
909+
shape = RoundedCornerShape(CHIP_CORNER_DP.dp),
910+
)
911+
.indication(interactionSource, ripple())
912+
.padding(horizontal = CHIP_HORIZONTAL_PAD_DP.dp),
913+
) {
914+
Text(
915+
text = label,
916+
color = textColor,
917+
fontSize = CHIP_FONT_SP.sp,
918+
fontWeight = FontWeight.Medium,
919+
letterSpacing = CHIP_LETTER_SPACING_SP.sp,
920+
)
921+
}
840922
}
841923
}
842924

android/Gutenberg/src/main/java/org/wordpress/gutenberg/inserter/PhotoAccessState.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,22 @@ import android.net.Uri
99
* `shouldShowRequestPermissionRationale` alone can't tell those apart.
1010
*/
1111
internal sealed interface PhotoAccess {
12-
data class Granted(val uris: List<Uri>) : PhotoAccess
12+
/**
13+
* Permission has been granted. `partialAccess` is non-null on Android 14+
14+
* when the user picked "Select photos and videos" rather than full access —
15+
* its `onManageSelection` reopens the system picker so the user can update
16+
* the selection without leaving the app.
17+
*/
18+
data class Granted(
19+
val uris: List<Uri>,
20+
val partialAccess: PartialAccess? = null,
21+
) : PhotoAccess
1322
data class NeedsPermission(
1423
val state: PromptState,
1524
val request: () -> Unit,
1625
) : PhotoAccess
26+
27+
data class PartialAccess(val onManageSelection: () -> Unit)
1728
}
1829

1930
/**

0 commit comments

Comments
 (0)