Page MenuHomePhabricator

Fix "choose icon" on profile menu items
ClosedPublic

Authored by epriestley on Jan 21 2016, 6:04 PM.
Tags
None
Referenced Files
F14402480: D15073.id36396.diff
Mon, Dec 23, 12:39 AM
Unknown Object (File)
Thu, Dec 19, 12:59 AM
Unknown Object (File)
Sat, Dec 14, 3:44 AM
Unknown Object (File)
Sun, Dec 8, 11:31 AM
Unknown Object (File)
Mon, Nov 25, 8:01 AM
Unknown Object (File)
Sun, Nov 24, 2:14 AM
Unknown Object (File)
Nov 21 2024, 3:40 AM
Unknown Object (File)
Nov 16 2024, 9:09 PM
Subscribers
None

Details

Summary

Ref T10054. This fix is a little rough but the "right" fix involves a ton of rewriting to AphrontSideNavFilterView and I don't want to open that can of worms up yet.

Specifically, the problem is:

  • we build the menu in order to populate the mobile/application menu;
  • as a side effect of building the menu (not rendering the menu), we initialize the menu collapse/expand behavior;
  • but we never actually render the menu, so the JX.$() call fails.

The right fix would be to initialize the behavior only when we render the menu, but then AphorntSideNavFilterView would need to know about profile menu behaviors. It probably should some day, but I think today is not that day.

Test Plan

Set icons on a link on a profile menu.

Diff Detail

Repository
rP Phabricator
Branch
pmenuicon1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10232
Build 12439: Run Core Tests
Build 12438: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Fix "choose icon" on profile menu items.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.Jan 21 2016, 6:05 PM
This revision was automatically updated to reflect the committed changes.