Skip to content

RS Post Settings: Device gallery as featured image source#22690

Merged
nbradbury merged 32 commits intotrunkfrom
feature/rsposts-device-featured-image
Mar 17, 2026
Merged

RS Post Settings: Device gallery as featured image source#22690
nbradbury merged 32 commits intotrunkfrom
feature/rsposts-device-featured-image

Conversation

@nbradbury
Copy link
Copy Markdown
Contributor

@nbradbury nbradbury commented Mar 13, 2026

Description

Adds the ability to pick a featured image from the device gallery (in addition to the existing WP Media Library option). When a device image is selected, it is uploaded to the site's media library and then set as the featured image.

Key changes:

  • Split featured image picker into two options: "Choose from WP Media" and "Choose from device" via a dropdown menu
  • Upload flow: copies the selected URI to a temp file, uploads it via MediaCreateParams, and sets the returned media ID as the featured image

Testing instructions

Featured image from WP Media:

  1. Open a post's RS settings (requires experimental feature flag)
  2. Tap the edit button on the featured image area
  3. Select "Choose from WP Media"
  • Verify the WP Media picker opens
  1. Select an image
  • Verify the featured image updates

Featured image from device:

  1. Open a post's RS settings
  2. Tap the edit button on the featured image area
  3. Select "Choose from device"
  • Verify the device media picker opens
  1. Select a photo
  • Verify a loading state appears while uploading
  • Verify the featured image updates after upload completes

Remove featured image:

  1. With a featured image set, tap the edit button
  2. Select "Remove"
  • Verify the featured image is removed
featured.mp4

nbradbury and others added 20 commits March 12, 2026 06:58
Replace Toast with Compose Snackbar for inline error messages with retry
actions, add pull-to-refresh support, show "Saving..." label alongside
the spinner, display "Not Set" placeholder for empty status, and improve
accessibility with content descriptions for password toggle and featured
image placeholder. Extract shared error utilities from PostRsListViewModel
into PostRsErrorUtils for reuse across both list and settings screens.
Add unit tests for PostRsSettingsViewModel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Preserve unsaved edits when refreshing post from server
- Remove unreachable snackbar when site is null (activity finishes immediately)
- Fix PullToRefreshBox content indentation
- Remove redundant offline mocks in refresh tests
- Rename misleading test name for status selection
- Add test for save-online sets isSaving

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract preserveEdits() helper to shorten refreshPost() below the
60-line detekt limit, and remove empty line after opening brace in
test class.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add navigationBarsPadding to SnackbarHost so snackbars aren't
obscured by the system navigation bar. Wrap ErrorContent in a
centered Box so the error message displays in the center of the
screen instead of at the top.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add fillMaxWidth and TextAlign.Center to the ErrorContent on the
term selection screen so the network error message is properly
centered horizontally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The network error snackbar shown when saving in airplane mode was
missing an actionLabel and onAction callback, so no Retry button
appeared. Add both so the user can retry saving after reconnecting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set the snackbar action color to MaterialTheme.colorScheme.primary
so the Retry text on snackbars matches the primary color used by
the Retry button on the error empty state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…race condition

When disabling airplane mode and immediately tapping Retry, Android may not
have re-established connectivity yet, causing a spurious network error. Now
the save always proceeds to the API call, which handles network errors
naturally via its catch block using PostRsErrorUtils.friendlyErrorMessage().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use BackHandler(onBack=) instead of wrapping lambda
- Use ?.let { } ?: Modifier for conditional click modifiers
- Replace PostApiException with RuntimeException
- Inline isAuthError wrapper in PostRsListViewModel
- Remove status bar hiding to fix PTR triggering on system bar pull

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace RuntimeException with PostApiRequestException to fix
TooGenericExceptionThrown detekt violations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On self-hosted sites, a new WpApiClient was created for every API call
(5 instantiations just to open post settings). Cache self-hosted clients
in WpApiClientProvider (mirroring the existing WP.com pattern), add a
per-site client cache in PostRsRestClient, and use a lazy property in
PostRsSettingsViewModel to reuse the same client across fetch and save.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Now that WpApiClientProvider caches self-hosted clients, the per-site
client cache in PostRsRestClient is redundant — remove it. Also remove
the now-unused site parameters from fetchPost() and savePost() in the
ViewModel, and replace shadowed locals with simple null guards.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eption, remove unused methods

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show hero image shimmer and placeholder rows while loading instead
of a titled app bar with a centered spinner.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract shared HeroOverlay composable to deduplicate gradient
  and back button between loading skeleton and hero layout
- Inline statusResId variable
- Remove unreachable site null guards in ViewModel
- Remove extra blank line in WpApiClientProvider

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allow choosing a featured image from the device gallery in addition
to the WP Media Library. Device picks are copied to a temp file and
uploaded via the wordpress-rs media API before being set as the
featured image.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e options

Replace the single "Change" menu item with "Choose from WP Media" and
"Choose from device" options in the featured image dropdown. The placeholder
(no image set) now shows the same dropdown menu instead of jumping straight
to the picker. Each option launches its own dedicated picker without
confusing source-switching toolbars.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented Mar 13, 2026

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@nbradbury nbradbury changed the base branch from trunk to feature/rsposts-settings-performance March 13, 2026 17:17
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Mar 13, 2026

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22690-4ab66b1
Build Number1487
Application IDorg.wordpress.android.prealpha
Commit4ab66b1
Installation URL0srch039315uo
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Mar 13, 2026

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22690-4ab66b1
Build Number1487
Application IDcom.jetpack.android.prealpha
Commit4ab66b1
Installation URL339ssc4em7vvg
Note: Google Login is not supported on these builds.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 0.76336% with 130 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.58%. Comparing base (f662c9e) to head (4ab66b1).
⚠️ Report is 4 commits behind head on trunk.

Files with missing lines Patch % Lines
...ress/android/ui/postsrs/PostRsSettingsViewModel.kt 1.66% 59 Missing ⚠️
...android/ui/postsrs/screens/PostRsSettingsScreen.kt 0.00% 37 Missing ⚠️
...rg/wordpress/android/ui/postsrs/UriToFileMapper.kt 0.00% 32 Missing ⚠️
...dpress/android/ui/postsrs/PostRsSettingsUiState.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22690      +/-   ##
==========================================
- Coverage   37.61%   37.58%   -0.04%     
==========================================
  Files        2272     2273       +1     
  Lines      118379   118477      +98     
  Branches    16370    16382      +12     
==========================================
+ Hits        44531    44532       +1     
- Misses      70196    70293      +97     
  Partials     3652     3652              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from feature/rsposts-settings-performance to trunk March 16, 2026 14:59
nbradbury and others added 4 commits March 16, 2026 11:08
…ce-featured-image

# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/postsrs/PostRsSettingsViewModel.kt
#	WordPress/src/main/java/org/wordpress/android/ui/postsrs/screens/PostRsSettingsScreen.kt
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nbradbury nbradbury marked this pull request as ready for review March 16, 2026 17:16
@nbradbury nbradbury requested a review from adalpari March 16, 2026 17:17
@adalpari
Copy link
Copy Markdown
Contributor

adalpari commented Mar 17, 2026

Overall, it looks and works well.

What do you think about the following points?

  • Temp file cleanup is missing: In copyUriToTempFile() (line ~540), a temp file is created in appContext.cacheDir but
    is never deleted after the upload completes (or fails). Over time, repeated picks will accumulate orphan files.
    Consider deleting the temp file in a finally block inside onFeaturedImagePickedFromDevice:

    } finally {
    tempFile?.delete()
    }

  • Hardcoded .jpg extension (PostRsSettingsViewModel.kt:~536): The temp file always gets a .jpg suffix regardless of the actual image type (e.g., PNG, WEBP, HEIC). If the upload API or server infers MIME type from the extension, this could cause issues. Consider reading the MIME type from the ContentResolver and mapping it to the correct extension.

  • @ApplicationContext in ViewModel (PostRsSettingsViewModel.kt:50): Injecting Context into a ViewModel is generally discouraged in Android architecture guidelines because it makes testing harder and blurs layer boundaries. Consider extracting the copyUriToTempFile logic into a small injectable helper/use-case class (e.g., MediaFileProvider or UriToFileMapper) that holds the Context. This would also make the file operations independently testable.

nbradbury and others added 6 commits March 17, 2026 06:43
The temp file created by copyUriToTempFile() was never cleaned up,
causing orphan files to accumulate in the cache directory. Add a
finally block to delete the temp file after upload completes or fails.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The temp file for device-picked featured images was always created
with a .jpg extension regardless of actual image type. Use
ContentResolver and MimeTypeMap to determine the correct extension,
falling back to .jpg when the MIME type is unavailable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move copyUriToTempFile and its Context dependency out of the
ViewModel into a dedicated injectable helper. This improves
testability and removes the @ApplicationContext from the ViewModel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update PostRsSettingsViewModelTest to match the ViewModel constructor
change from the UriToFileMapper extraction commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lename for upload title

Delete the temp file if the stream copy fails so partial files don't
linger. Query the ContentResolver for the original display name and
use it as the media upload title instead of the temp file name. Also
fix import ordering in PostRsSettingsActivity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Narrow catch from Exception to IOException to satisfy detekt's
TooGenericExceptionCaught rule. Add @Suppress("Recycle") to
getDisplayName since the cursor is already closed by .use {}.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nbradbury
Copy link
Copy Markdown
Contributor Author

What do you think about the following points?

@adalpari Those are all good points and I've fixed them.

Handle the Boolean return value of File.delete() by logging a
warning when deletion fails. Extract the duplicated "No site
selected" string into a companion object constant.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@nbradbury nbradbury enabled auto-merge (squash) March 17, 2026 11:44
Copy link
Copy Markdown
Contributor

@adalpari adalpari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fixes! LGTM!

@nbradbury nbradbury merged commit 8265755 into trunk Mar 17, 2026
24 checks passed
@nbradbury nbradbury deleted the feature/rsposts-device-featured-image branch March 17, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants