Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Self-hosted user UI #21275

Merged
merged 71 commits into from
Oct 7, 2024
Merged

Self-hosted user UI #21275

merged 71 commits into from
Oct 7, 2024

Conversation

nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Oct 3, 2024

In this PR, I've combined #21264 (self-hosted user list) and #21270 (self-hosted user detail). This did make for a larger PR, but it should be easier to review than having separate PRs, plus a lot of the code is Jetpack Compose which can be quite verbose.

To test:

  • Enable self_hosted_users in debug settings
  • Switch to a self-hosted site
  • Note that a new "Users" item appears
  • Tap on that and verify the list screen with dummy data looks fine
  • Tap a user to view the detail screen and verify that looks fine
  • If the user has an avatar, tap it and verify the large avatar view looks fine
  • Switch to a non-self-hosted site and verify the "Users" item doesn't appear
  • View the various Compose previews of the SelfHostedUsersScreen and verify they look fine (note that remote avatar images won't load in the preview)

Caveats:

  • This is a UI-only PR - no network requests are made, instead only dummy data is used
  • The UI is somewhat different than in this iOS PR but it's closer to the appearance of the existing People feature on Android
users.mp4

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 3, 2024

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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 3, 2024

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

@@ -724,6 +725,13 @@
AnalyticsUtils.trackWithSiteDetails(AnalyticsTracker.Stat.OPENED_PEOPLE_MANAGEMENT, site);
}

public static void viewSelfHostedUsers(Context context, SiteModel site) {

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on method parameter Note

Missing null annotation
@@ -724,6 +725,13 @@
AnalyticsUtils.trackWithSiteDetails(AnalyticsTracker.Stat.OPENED_PEOPLE_MANAGEMENT, site);
}

public static void viewSelfHostedUsers(Context context, SiteModel site) {

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on method parameter Note

Missing null annotation
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 1.24378% with 397 lines in your changes missing coverage. Please review.

Project coverage is 40.24%. Comparing base (625ee74) to head (f53dcf6).
Report is 72 commits behind head on trunk.

Files with missing lines Patch % Lines
...ndroid/ui/selfhostedusers/SelfHostedUsersScreen.kt 0.00% 178 Missing ⚠️
...id/ui/selfhostedusers/SelfHostedUserComposables.kt 0.00% 108 Missing ⚠️
...ordpress/android/ui/selfhostedusers/SampleUsers.kt 0.00% 64 Missing ⚠️
...oid/ui/selfhostedusers/SelfHostedUsersViewModel.kt 0.00% 36 Missing ⚠️
...id/ui/mysite/items/listitem/SiteListItemBuilder.kt 33.33% 6 Missing ⚠️
...ava/org/wordpress/android/ui/ActivityLauncher.java 0.00% 3 Missing ⚠️
...ordpress/android/ui/mysite/SiteNavigationAction.kt 0.00% 1 Missing ⚠️
...s/android/ui/mysite/cards/ListItemActionHandler.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #21275      +/-   ##
==========================================
- Coverage   40.46%   40.24%   -0.23%     
==========================================
  Files        1531     1535       +4     
  Lines       69831    70232     +401     
  Branches    11444    11545     +101     
==========================================
+ Hits        28259    28262       +3     
- Misses      39125    39523     +398     
  Partials     2447     2447              

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

@nbradbury nbradbury marked this pull request as ready for review October 3, 2024 13:46
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

Nice! Exciting to see this feature begin to take shape. The testing instructions passed for me using a Samsung Galaxy S20 running Android 13.

I left a few suggestions to consider, but I do not consider them blockers. Let me know what you think.

Copy link

sonarcloud bot commented Oct 7, 2024

@nbradbury nbradbury requested review from dcalhoun and removed request for geriux October 7, 2024 15:44
@wordpress-mobile wordpress-mobile deleted a comment from nealgoogs Oct 7, 2024
@nbradbury nbradbury merged commit 1281a8b into trunk Oct 7, 2024
21 checks passed
@nbradbury nbradbury deleted the issue/self-hosted-user-ui branch October 7, 2024 16:00
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