Page MenuHomePhorge

Allow admins to access "Manage" page from user profile page menu on mobile
AbandonedPublic

Authored by aklapper on Jun 7 2024, 17:12.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jul 13, 23:55
Unknown Object (File)
Sat, Jul 13, 23:33
Unknown Object (File)
Thu, Jul 11, 14:55
Unknown Object (File)
Wed, Jul 10, 07:48
Unknown Object (File)
Wed, Jul 10, 07:17
Unknown Object (File)
Sat, Jul 6, 21:57
Unknown Object (File)
Wed, Jul 3, 05:15
Unknown Object (File)
Wed, Jul 3, 04:54

Details

Summary

Make the mobile hamburger menu on the User Profile page more useful at least for admins by adding a menu item to access the Manage page for that user (e.g. for disabling the user account or making them an administrator).

See also:

Test Plan

As an admin, go to a user profile page like http://phorge.localhost/username/ on a screen with 920px or less width and click the hamburger menu item in the upper right corner. Click the "Manage $username" item to end up on a page which offers an "Actions" button which allows you to access actions like "Disable User" or "Make Administrator" also on mobile now and not only on desktop anymore. Great for disabling accounts on the go when a full PC computer would not come handy!

Diff Detail

Repository
rP Phorge
Branch
peopleMenuMobileManage (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1335
Build 1335: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.Jun 7 2024, 17:12
aklapper retitled this revision from Allow to access "Manage" page from user profile page menu on mobile to Allow admins to access "Manage" page from user profile page menu on mobile.Jun 7 2024, 17:12

Better than before.

In an ideal world we should make this manual menu nuked, and simply auto-generate the mobile menu with the same things as the left desktop menu (F311948).

src/applications/people/controller/PhabricatorPeopleController.php
28

✅ The PHP pitfall that skips zeroes is under control here. Since ID 0 is not a meaningful value.

This revision is now accepted and ready to land.Jun 7 2024, 19:33

As T15224 suggest, this menu should have the same content as the side-menu on non-mobile view; And in most places, the framework handles that, by actually moving that menu.
So the "right" fix is to find what creates the current side-menu and make sure we call that, or figuring out why they're different here and normally the same.

I think @valerio.bozzolan did a similar thing to Home a while back?

this menu should have the same content as the side-menu on non-mobile view; And in most places, the framework handles that, by actually moving that menu.
So the "right" fix is to find what creates the current side-menu and make sure we call that, or figuring out why they're different here and normally the same.

Completely agree but I don't know how 🤔

I think @valerio.bozzolan did a similar thing to Home a while back?

Ah! That T15216! It was awesome! Unfortunately that was an easy CSS bug.

So the "right" fix is to find what creates the current side-menu and make sure we call that, or figuring out why they're different here and normally the same.

@avivey I agree; ideally I'd also like to see the same menu entries without having to manually define entries for the mobile menu. However for now I cannot put time/priority into investigating how to fix this perfectly.
Our downstream usecase is allowing our admins on their mobile phones to disable users without having to manually construct a URL.
I'd be happy to see a "perfect" fix as a followup contribution. For now I consider my fix "good enough" to allow any Phorge admins out there on mobile phones to reach the menu to manage user accounts in their installation. Does that make sense?

To me it makes sense thanks. Maybe just add a comment somewhere to say that the menu should be automatically generated - T15224

There is no reason to expose "Manage" for admins only. I'm abandoning this in favor of D25687.

On a related note, https://we.phorge.it/source/phorge/browse/master/src/applications/diffusion/controller/DiffusionRepositoryManagePanelsController.php says TODO: This is messy for now; the mobile menu should be set automatically in buildApplicationMenu().

Also, grep'ing the source I find both buildSideNavView and buildSideNav functions in classes which extends PhabricatorController. They seem to do the same, so there is some inconsistency here.