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:
Details
Details
- Reviewers
valerio.bozzolan • Wikivu - Group Reviewers
O1: Blessed Committers
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
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
Comment Actions
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. | |
Comment Actions
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?
Comment Actions
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.
Comment Actions
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?
Comment Actions
To me it makes sense thanks. Maybe just add a comment somewhere to say that the menu should be automatically generated - T15224
Comment Actions
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.