Ref T12949, Fix profile menu link selection highlighting.
Details
- Reviewers
lpriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T12949: If a ProfileMenu has Link items, and one or more have the request URI as the link target, highlight the first one
- 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
- Repository
- rP Phabricator
- Branch
- profilemenufilterselection
- Lint
Lint Passed - Unit
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:
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.
src/applications/search/engine/PhabricatorProfileMenuEngine.php | ||
---|---|---|
239 | Selection occurs here. |