My site fragment: no sites empty view

[nbradbury]

As part of CMM-1037, this PR removes the image from the My Site fragment's empty view, which is shown when there are no sites.

Note1: I've added a TODO regarding test code which ensures that regardless of the account, the empty view appears. I'll remove this before merging. This is why the "Do not merge" label has been added.

Note2: I'm not convinced removing the image in this situation is the right thing to do. I know we're trying to get rid of outdated images in the various empty views, but not having any sites seems a special situation that may warrant keeping the image. Thoughts?

To test
Simply run the app and verify the empty view on the My Site page appears correctly.

Before and After

[dangermattic]

	1 Error
🚫	This PR is tagged with Do Not Merge label(s).

	1 Warning
⚠️	PR is not assigned to a milestone.

Generated by 🚫 Danger

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[nbradbury]

@claude Ignore the "TODO" and the "Do not merge" label.

[claude]

Claude finished @nbradbury's task in 57s —— View job

---

Code Review

Reviewing PR changes (ignoring TODO and "Do not merge" label as requested).

Tasks

• Read changed files to understand modifications
• Perform comprehensive code review
• Provide detailed feedback

---

Review Summary

I've reviewed the changes in this PR. The modifications are straightforward and handle the image removal correctly. Here are my findings:

✅ Code Quality - Excellent

The changes are minimal and focused:

1. XML Layout (my_site_fragment_no_sites_view.xml:13): Removed the app:aevImage attribute from the ActionableEmptyView
2. Kotlin Fragment (MySiteFragment.kt:499-501): Added test code with TODO (as noted, this will be removed)
3. Code formatting fix (MySiteFragment.kt:503): Fixed spacing in the if statement

The ActionableEmptyView component handles missing images gracefully - when imageResource == 0, it simply doesn't set or show the image (ActionableEmptyView.kt:85-90). No null pointer or visibility issues will occur.

✅ Security - No Issues

No security concerns. This is a UI-only change with no security implications.

✅ Performance - No Issues

Removing the image actually improves performance slightly by:

• Reducing layout inflation time
• Eliminating image loading operations
• Reducing memory usage

⚠️ Testing - Minor Observation

The test code (lines 499-501) forces the empty view to always display, which is intentional for testing purposes. This correctly simulates the "no sites" state regardless of actual account status.

Note: As mentioned in the PR description, this TODO code will be removed before merge.

📝 Documentation - Consider Adding Context

While the code changes are correct, you might want to consider:

1. Design Decision Documentation: Since you mentioned uncertainty about removing the image ("not convinced this is the right thing to do"), it might be helpful to document the reasoning if this gets merged. This could be:

   • A comment in the layout file explaining why no image is used
   • Or a note in the commit message for future reference

2. Related Empty Views: If this is part of a broader initiative to remove outdated images from empty views (as mentioned in the PR description), you might want to ensure consistency across similar empty states.

Code Style Compliance

The changes follow the project's code style guidelines:

• ✅ Line length under 120 characters
• ✅ Proper Kotlin formatting
• ✅ No deprecated APIs used
• ✅ Uses TODO instead of FIXME

Conclusion

The implementation is clean and correct. The ActionableEmptyView component was designed to handle optional images, so removing the app:aevImage attribute is safe and won't cause any runtime issues. The test code clearly marked with TODO serves its purpose for verification.

The only open question is the design decision itself (whether to remove the image), which you've already flagged for discussion. From a technical implementation standpoint, this PR is solid.

---

[wpmobilebot]

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

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22503-633a812
	Commit	633a812
	Direct Download	jetpack-prototype-build-pr22503-633a812.apk

Note: Google Login is not supported on these builds.

[wpmobilebot]

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

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22503-633a812
	Commit	633a812
	Direct Download	wordpress-prototype-build-pr22503-633a812.apk

Note: Google Login is not supported on these builds.

[wpmobilebot]

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

[nbradbury]

This build has failures.

This is due to the "Do not merge" label.

[adalpari]

Note2: I'm not convinced removing the image in this situation is the right thing to do. I know we're trying to get rid of outdated images in the various empty views, but not having any sites seems a special situation that may warrant keeping the image. Thoughts?

I do agree. I'm not sure that opening the app and seeing that plain message is a pleasant UX...
My vote is on keeping the illustration in this case, and maybe add a note to the code about why we think it's better keeping it.

Anyway, I'm approving the PR, so you can go with whatever you think is better.

[adalpari]

LGTM