Page MenuHomePhabricator

WIP Menu item that is a link to a state of a builtin menu item should display consistent selection behavior

Authored by epriestley on Dec 7 2017, 10:18 PM.



Ref T12949, Fix profile menu link selection highlighting.

Test Plan
  • Open project where one of the menu items is a link to a filtered workboad view
  • Open that link

Link should be properly highlighted when selected

Diff Detail

rP Phabricator
Lint Passed
Tests Passed
Build Status
Buildable 18935
Build 25534: Run Core Tests
Build 25533: arc lint + arc unit

Event Timeline

So this would work in theory, but the problem is that the buildNavigation() method in PhabricatorProfileMenuEngine.php is called twice. The first time, the selection is properly set, but the second time around, it's reset back to the original buggy behavior. Wanted to know if I'm generally in the right direction or if I should pivot before it's too late.

I think navigation is built once for the page itself, and once for the mobile/tablet action menu:

Screen Shot 2017-12-14 at 2.01.32 PM.png (711×852 px, 141 KB)

For the main page, the selection is overwritten explicitly (see inline). For the mobile version, items should not be selected.

Looking at this further, selection on the board page is currently performed explicitly inside PhabricatorProjectBoardController, and project profile page selection is currently performed explicitly inside PhabricatorProjectProfileController.

I think this change probably requires a significant amount of refactoring and clarification of how ProfileMenu works. Some newer items, like PhabricatorDashboardProfileMenuItem, implement their content inside the item (via newPageContent()). These items can more realistically have their selection behavior modified in a global way. But older items, including most of projects, implement content in top-level Controller classes and just use menu items in a dumb way.

A hacky way to implement this would be to implement some selectFilterFromRequest($request) method on AphrontSideNavFilterView, then copy/paste a bunch of calls to it into PhabricatorProjectBoardController, PhabricatorProjectProfileController, PhabricatorProfileMenuEngine, and so on. However, I think the value of this behavior isn't worth the mess that would create. This will probably be refactored at some point, maybe if/when user profiles become driven by ProfileMenu.


Selection occurs here.

This revision now requires changes to proceed.Dec 14 2017, 10:18 PM
epriestley abandoned this revision.
epriestley edited reviewers, added: lpriestley; removed: epriestley.

Approximately obsoleted by the very messy patch in D20353. See also T13275.