Skip to content

Mediagrid outdated empty state#22391

Merged
nbradbury merged 9 commits intotrunkfrom
issue/CMM-1037-mediagrid-outdated-empty-state
Dec 5, 2025
Merged

Mediagrid outdated empty state#22391
nbradbury merged 9 commits intotrunkfrom
issue/CMM-1037-mediagrid-outdated-empty-state

Conversation

@nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Dec 3, 2025

As part of CMM-1037, this small PR updates the media grid's empty view by:

  • Removing the outdated image
  • Making the title text more lively
  • Adding a subtitle

I also cleaned up some of the search logic.

To test

  • Remove all media from a test site then view the site media in the app
  • Verify that the empty view works as expected
  • Verify that empty search results works as expected
  • Switch to a site that has media and verify it works as expected

Before

MediaGridFragment-before

After

MediaGridFragment-after

@nbradbury
Copy link
Contributor Author

@claude

@dangermattic
Copy link
Collaborator

dangermattic commented Dec 3, 2025

2 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@claude
Copy link
Contributor

claude bot commented Dec 3, 2025

Claude finished @nbradbury's task —— View job


Code Review ✅

  • Gather context and read changed files
  • Analyze PR changes
  • Review code quality and best practices
  • Provide feedback

Review Summary

This is a clean, focused UI improvement for the media grid's empty state. The changes successfully modernize the empty view by making it more engaging and action-oriented.

✅ Strengths

1. UI/UX Improvements

  • Removed outdated image: Good decision to remove app:aevImage="@drawable/img_illustration_media_105dp". Modern empty states often work better with minimal visual elements, letting the text be the focal point.
  • More engaging title: The change from "You don't have any media" to "You don't have any media - yet!" adds optimism and implies future action, which is excellent UX copy.
  • Added helpful subtitle: "Why not upload something?" provides clear next steps and encourages user action.

2. Code Quality

  • Minimal, focused change that does exactly what it needs to
  • Proper use of string resources for localization
  • Clean XML formatting and structure

3. Consistency

  • The subtitle uses the existing app:aevSubtitle attribute, which is the correct way to add this content to an ActionableEmptyView
  • Follows the existing pattern used elsewhere in the codebase

📝 Minor Observations

String Resource Location (strings.xml:228-229)

  • The new strings are properly placed near related media empty state strings, maintaining good organization
  • String names follow the existing convention (media_empty_list and media_empty_list_subtitle)

Potential Consideration (not blocking)

  • Consider whether the enthusiastic tone ("- yet!" and "Why not upload something?") aligns with the overall app voice. It's more casual/playful than some other empty states. If this is part of a broader UX refresh, it's perfect!

🎯 Recommendations

None - This PR is ready to merge as-is. The changes are:

  • ✅ Small and focused
  • ✅ Improve user experience
  • ✅ Follow Android best practices
  • ✅ Properly use string resources for localization
  • ✅ Don't introduce any security, performance, or maintainability concerns

The before/after screenshots clearly show the improvement. Nice work! 🎉


@nbradbury nbradbury marked this pull request as ready for review December 3, 2025 21:29
@nbradbury nbradbury requested review from adalpari and removed request for adalpari December 3, 2025 21:29
@nbradbury nbradbury marked this pull request as draft December 3, 2025 21:30
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 3, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
FlavorJalapeno
Build TypeDebug
Versionpr22391-b0c3c1b
Commitb0c3c1b
Direct Downloadjetpack-prototype-build-pr22391-b0c3c1b.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 3, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
FlavorJalapeno
Build TypeDebug
Versionpr22391-b0c3c1b
Commitb0c3c1b
Direct Downloadwordpress-prototype-build-pr22391-b0c3c1b.apk
Note: Google Login is not supported on these builds.

@nbradbury
Copy link
Contributor Author

@claude

@claude
Copy link
Contributor

claude bot commented Dec 4, 2025

Claude finished @nbradbury's task —— View job


Code Review


Tasks

  • Gather context and read changed files
  • Analyze PR changes against review criteria
  • Provide comprehensive feedback

Review Summary

This PR successfully modernizes the media grid's empty state with improved UX copy and better search handling. The changes are clean, focused, and well-implemented. A few observations and minor suggestions below.


✅ Strengths

1. UX Improvements

  • More engaging copy: Adding "- yet!" and "Why not upload something?" transforms a negative empty state into an encouraging, action-oriented one. This is a significant UX improvement.
  • Removed outdated image: The removal of app:aevImage="@drawable/img_illustration_media_105dp" makes the empty state cleaner and more modern.
  • Subtitle addition: The new subtitle provides clear guidance on what users should do next.

2. Code Quality

  • Proper separation of concerns: The new onSearchClosed() method (MediaGridFragment.java:617-625) properly encapsulates search-closing logic that was previously scattered.
  • Better empty view handling: The search flow now properly shows/hides the empty view based on whether there are search results (MediaGridFragment.java:610-614).
  • Consistent string resource usage: All new strings are properly externalized for localization.
  • Type safety: Good use of @StringRes annotations for the title and subtitle IDs (MediaGridFragment.java:660-661).

3. Bug Fix

  • Fixed search close behavior: The refactoring in MediaBrowserActivity.java:728 properly delegates to onSearchClosed() instead of just showing the button. This ensures the fragment's internal state is properly updated when search closes.

📝 Observations & Suggestions

1. String Resources (strings.xml:228-234)

  • The warning from Danger bot about updating strings.xml on non-release branches is worth noting. The PR author may need to handle this during the merge process.
  • The enthusiastic tone ("- yet!", "Why not...?") is more casual than some other parts of the app. If this is part of a broader UX voice refresh, it's excellent. If not, consider whether this tone is consistent with the app's overall voice.

2. Subtitle Visibility Logic (MediaGridFragment.java:707-712)

  • The subtitle is only shown for the NO_CONTENT case when not searching. This is correct behavior, but worth documenting since it's a conditional display.
  • Consider if the subtitle should appear for other media types (images-only, videos-only, etc.) or just the "all media" view. Currently it shows for all types, which seems appropriate.

3. Empty View State Management (MediaGridFragment.java:610-625)

  • The new onSearchClosed() method properly manages the empty view visibility and button state. This is a good refactoring.
  • The logic flow is: search closed → clear search term → check if empty → show empty view with button OR hide empty view.
  • This is more robust than the previous implementation.

4. Image Visibility (MediaGridFragment.java:672)

  • The code explicitly sets mActionableEmptyView.image.setVisibility(View.VISIBLE) for non-search empty states.
  • However, since the image drawable was removed from the XML layout, this visibility setting might be unnecessary unless there's a default image being shown by the ActionableEmptyView component.
  • Minor point: You might want to verify what the visual result is here.

5. Line Length (MediaGridFragment.java:669)

  • Line 669 in the original file has mActionableEmptyView.updateLayoutForSearch(true, 0); which is fine.
  • The CLAUDE.md file emphasizes keeping lines under 120 characters. All changes respect this limit. ✅

🔍 Testing Considerations

The PR description provides good test steps:

  • ✅ Remove all media and verify empty view
  • ✅ Verify empty search results
  • ✅ Switch to site with media and verify

Additional test scenarios to consider:

  1. Search flow: Enter search → No results → Close search → Verify empty view appears with subtitle and button
  2. Different filters: Switch between All/Images/Videos/Documents/Audio tabs when empty to verify all show the "- yet!" message
  3. Screen rotation: Verify empty state persists correctly through configuration changes
  4. Accessibility: Test with TalkBack to ensure the new subtitle is announced properly

🎯 Security, Performance & Best Practices

Security: ✅ No security concerns. String resources are properly escaped.

Performance: ✅ No performance impact. The changes are purely UI updates with no additional processing.

Best Practices:

  • ✅ Proper use of string resources for localization
  • ✅ Proper use of visibility constants (View.VISIBLE, View.GONE)
  • ✅ Good separation of concerns with the new onSearchClosed() method
  • ✅ Consistent code style with existing codebase
  • ✅ No deprecated APIs used

Testing: ⚠️ No new tests added. While this is a small UI change, consider adding:

  • Unit tests for the onSearchClosed() method to verify it properly clears search term and manages empty view state
  • UI tests to verify the subtitle appears in the empty state

📋 Final Verdict

✅ Approved - This PR is well-implemented and ready to merge. The changes improve user experience with minimal code changes, properly handle edge cases, and follow Android best practices.

Minor Recommendations (not blocking):

  1. Consider adding unit tests for the new onSearchClosed() method
  2. Verify the image.setVisibility(View.VISIBLE) behavior since the image drawable was removed
  3. Ensure the enthusiastic tone aligns with the broader app voice guidelines

The before/after screenshots clearly demonstrate the improvement. Nice work! 🎉


@nbradbury nbradbury marked this pull request as ready for review December 4, 2025 17:01
@nbradbury nbradbury requested a review from adalpari December 4, 2025 17:01
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 4, 2025

Copy link
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.

Looks great!

@nbradbury nbradbury merged commit 9938dc1 into trunk Dec 5, 2025
24 checks passed
@nbradbury nbradbury deleted the issue/CMM-1037-mediagrid-outdated-empty-state branch December 5, 2025 11:24
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.

4 participants

Comments