Page MenuHomePhabricator

Make mobile navigation work properly by default in more cases

Authored by epriestley on Nov 2 2015, 8:20 PM.



Fixes T5752. This obsoletes a bunch of old patterns and I'll follow up on those with a big "go do a bunch of mechanical code changes" task. Major goals are:

  • Don't load named queries multiple times on search pages.
  • Don't require extra code to get standard navigation right on mobile.
  • Reduce the amount of boilerplate in ListControllers.
  • Reduce the amount of boilerplate around navigation/menus in all controllers.

Specifically, here's what this does:

  • The StandardPage is now a smarter/more structured object with setNavigation() and setCrumbs() methods. More rendering decisions are delayed until the last possible moment.
    • It uses this to automatically add crumb actions to the application menu.
    • It uses this to automatically reuse one SearchEngine instead of running queries multiple times.
  • The new preferred way to build responses is $this->newPage() (like $this->newDialog()), which has structured methods for adding stuff (setTitle(), etc).
  • SearchEngine exposes a new convenience method so you don't have to do all the controller delegation stuff.
  • Building menus is generally simpler.
Test Plan
  • Tested paste list, view, edit, comment, raw controllers for functionality, mobile menu, crumbs, navigation menu.
  • Edited saved queries.
  • Tested Differential, Maniphest (no changes).
  • Verified the paste pages don't run any duplicate NamedQuery queries.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Make mobile navigation work properly by default in more cases.
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.Nov 3 2015, 12:09 AM
This revision was automatically updated to reflect the committed changes.