Render Conpherence threads as a separate menu inside the layout, not a side nav

Summary: Currently, the thread list is in a standard side nav, but that makes it awkward to render (rendering logic needs to live in the base controller) and gives it some bad beahviors (like autohiding on mobile). Instead, move it into its own view and make it a little more custom. Ref T2421.

Test Plan: {F38098}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T2421

Differential Revision: https://secure.phabricator.com/D5504

src/__phutil_library_map__.php

@@ -242,6 +242,7 @@ phutil_register_library_map(array(
     'ConpherenceReplyHandler' => 'applications/conpherence/mail/ConpherenceReplyHandler.php',
     'ConpherenceSettings' => 'applications/conpherence/constants/ConpherenceSettings.php',
     'ConpherenceThread' => 'applications/conpherence/storage/ConpherenceThread.php',
+    'ConpherenceThreadListView' => 'applications/conpherence/view/ConpherenceThreadListView.php',
     'ConpherenceThreadQuery' => 'applications/conpherence/query/ConpherenceThreadQuery.php',
     'ConpherenceTransaction' => 'applications/conpherence/storage/ConpherenceTransaction.php',
     'ConpherenceTransactionComment' => 'applications/conpherence/storage/ConpherenceTransactionComment.php',
@@ -1956,6 +1957,7 @@ phutil_register_library_map(array(
       0 => 'ConpherenceDAO',
       1 => 'PhabricatorPolicyInterface',
     ),
+    'ConpherenceThreadListView' => 'AphrontView',
     'ConpherenceThreadQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'ConpherenceTransaction' => 'PhabricatorApplicationTransaction',
     'ConpherenceTransactionComment' => 'PhabricatorApplicationTransactionComment',

src/applications/conpherence/controller/ConpherenceController.php

@@ -5,26 +5,6 @@
  */
 abstract class ConpherenceController extends PhabricatorController {
   private $conpherences;
-  private $readConpherences;
-  private $unreadConpherences;
-
-  public function setUnreadConpherences(array $conpherences) {
-    assert_instances_of($conpherences, 'ConpherenceThread');
-    $this->unreadConpherences = $conpherences;
-    return $this;
-  }
-  public function getUnreadConpherences() {
-    return $this->unreadConpherences;
-  }
-
-  public function setReadConpherences(array $conpherences) {
-    assert_instances_of($conpherences, 'ConpherenceThread');
-    $this->readConpherences = $conpherences;
-    return $this;
-  }
-  public function getReadConpherences() {
-    return $this->readConpherences;
-  }
 
   /**
    * Try for a full set of unread conpherences, and if we fail
@@ -70,88 +50,22 @@ abstract class ConpherenceController extends PhabricatorController {
     $unread_conpherences = array_select_keys(
       $all_conpherences,
       array_keys($unread));
-    $this->setUnreadConpherences($unread_conpherences);
 
     $read_conpherences = array_select_keys(
       $all_conpherences,
       array_keys($read));
-    $this->setReadConpherences($read_conpherences);
 
-    return $this;
+    return array($unread_conpherences, $read_conpherences);
-  }
-
-  public function buildSideNavView($filter = null, $for_application = false) {
-    require_celerity_resource('conpherence-menu-css');
-    $unread_conpherences = $this->getUnreadConpherences();
-    $read_conpherences = $this->getReadConpherences();
-
-    $user = $this->getRequest()->getUser();
-    $menu = new PhabricatorMenuView();
-    $nav = AphrontSideNavFilterView::newFromMenu($menu);
-    $nav->addClass('conpherence-menu');
-    $nav->setMenuID('conpherence-menu');
-
-    if (!$for_application) {
-      $nav->addMenuItem(
-        id(new PhabricatorMenuItemView())
-          ->setName(pht('New Conversation'))
-          ->setWorkflow(true)
-          ->setKey('new')
-          ->setHref($this->getApplicationURI('new/'))
-          ->setType(PhabricatorMenuItemView::TYPE_BUTTON));
-
-      $nav->addLabel(pht('Unread'));
-      $nav = $this->addConpherencesToNav(
-        $unread_conpherences,
-        $nav,
-        false);
-      $nav->addLabel(pht('Read'));
-      $nav = $this->addConpherencesToNav(
-        $read_conpherences,
-        $nav,
-        true);
-      $nav->selectFilter($filter);
-    } else {
-      $nav->addFilter(
-        'new',
-        pht('New Conversation'),
-        $this->getApplicationURI('new/'));
-
-    }
-    return $nav;
-  }
-
-  private function addConpherencesToNav(
-    array $conpherences,
-    AphrontSideNavFilterView $nav,
-    $read = false) {
-
-    $user = $this->getRequest()->getUser();
-    foreach ($conpherences as $conpherence) {
-      $item = $this->buildConpherenceMenuItem($conpherence);
-
-      $nav->addCustomBlock($item->render());
-    }
-    if (empty($conpherences) || $read) {
-      $nav->addCustomBlock($this->getNoConpherencesBlock());
-    }
-
-    return $nav;
-  }
-
-  private function getNoConpherencesBlock() {
-    return phutil_tag(
-      'div',
-      array(
-        'class' => 'no-conpherences-menu-item'
-      ),
-      pht('No more conpherences.'));
   }
 
   public function buildApplicationMenu() {
-    return $this->buildSideNavView(
+    $nav = new PhabricatorMenuView();
-      $filter = null,
+
-      $for_application = true)->getMenu();
+    $nav->newLink(
+      pht('New Conversation'),
+      $this->getApplicationURI('new/'));
+
+    return $nav;
   }
 
   public function buildApplicationCrumbs() {
@@ -170,39 +84,6 @@ abstract class ConpherenceController extends PhabricatorController {
     return $crumbs;
   }
 
-  protected function buildConpherenceMenuItem($conpherence) {
-
-    $user = $this->getRequest()->getUser();
-    $uri = $this->getApplicationURI($conpherence->getID().'/');
-    $data = $conpherence->getDisplayData(
-      $user,
-      null);
-    $title = $data['title'];
-    $subtitle = $data['subtitle'];
-    $unread_count = $data['unread_count'];
-    $epoch = $data['epoch'];
-    $image = $data['image'];
-    $snippet = $data['snippet'];
-
-    $item = id(new ConpherenceMenuItemView())
-      ->setUser($user)
-      ->setTitle($title)
-      ->setSubtitle($subtitle)
-      ->setHref($uri)
-      ->setEpoch($epoch)
-      ->setImageURI($image)
-      ->setMessageText($snippet)
-      ->setUnreadCount($unread_count)
-      ->setID($conpherence->getPHID().'-nav-item')
-      ->addSigil('conpherence-menu-click')
-      ->setMetadata(
-        array(
-          'id' => $conpherence->getID(),
-          ));
-
-    return $item;
-  }
-
   protected function buildHeaderPaneContent(ConpherenceThread $conpherence) {
     $user = $this->getRequest()->getUser();
     $display_data = $conpherence->getDisplayData(

src/applications/conpherence/controller/ConpherenceListController.php

@@ -3,8 +3,8 @@
 /**
  * @group conpherence
  */
-final class ConpherenceListController extends
+final class ConpherenceListController
-  ConpherenceController {
+  extends ConpherenceController {
 
   private $conpherenceID;
 
@@ -45,23 +45,25 @@ final class ConpherenceListController extends
       $current_selection_epoch = $participant->getDateTouched();
     }
 
-    $this->loadStartingConpherences($current_selection_epoch);
+    list($unread, $read) = $this->loadStartingConpherences(
-    $nav = $this->buildSideNavView();
+      $current_selection_epoch);
+
+    $thread_view = id(new ConpherenceThreadListView())
+      ->setUser($user)
+      ->setBaseURI($this->getApplicationURI())
+      ->setUnreadThreads($unread)
+      ->setReadThreads($read);
 
     $main_pane = id(new ConpherenceLayoutView())
-      ->setBaseURI($this->getApplicationURI());
+      ->setBaseURI($this->getApplicationURI())
+      ->setThreadView($thread_view);
 
     if ($conpherence) {
       $main_pane->setThread($conpherence);
     }
 
-    $nav->appendChild(
-      array(
-        $main_pane,
-      ));
-
     return $this->buildApplicationPage(
-      $nav,
+      $main_pane,
       array(
         'title' => $title,
         'device' => true,

src/applications/conpherence/view/ConpherenceLayoutView.php

@@ -4,6 +4,11 @@ final class ConpherenceLayoutView extends AphrontView {
 
   private $thread;
   private $baseURI;
+  private $threadView;
+
+  public function getThreadView() {
+    return $this->threadView;
+  }
 
   public function setBaseURI($base_uri) {
     $this->baseURI = $base_uri;
@@ -15,6 +20,11 @@ final class ConpherenceLayoutView extends AphrontView {
     return $this;
   }
 
+  public function setThreadView(ConpherenceThreadListView $thead_view) {
+    $this->threadView = $thead_view;
+    return $this;
+  }
+
   public function render() {
 
     Javelin::initBehavior('conpherence-menu',
@@ -40,47 +50,67 @@ final class ConpherenceLayoutView extends AphrontView {
     return javelin_tag(
       'div',
       array(
-        'id' => 'conpherence-main-pane',
+        'sigil' => 'conpherence-layout',
-        'sigil' => 'conpherence-layout'
+        'class' => 'conpherence-layout',
       ),
       array(
         javelin_tag(
           'div',
           array(
-            'class' => 'conpherence-header-pane',
+            'class' => 'phabricator-nav-column-background',
-            'id' => 'conpherence-header-pane',
-            'sigil' => 'conpherence-header',
-          ),
-          ''),
-        phutil_tag(
-          'div',
-          array(
-            'class' => 'conpherence-widget-pane',
-            'id' => 'conpherence-widget-pane'
           ),
           ''),
         javelin_tag(
           'div',
           array(
-            'class' => 'conpherence-message-pane',
+            'class' => 'conpherence-menu-pane phabricator-side-menu',
-            'id' => 'conpherence-message-pane'
+            'sigil' => 'conpherence-menu-pane',
+          ),
+          nonempty($this->threadView, '')),
+        javelin_tag(
+          'div',
+          array(
+            'class' => 'conpherence-content-pane',
           ),
           array(
             javelin_tag(
               'div',
               array(
-                'class' => 'conpherence-messages',
+                'class' => 'conpherence-header-pane',
-                'id' => 'conpherence-messages',
+                'id' => 'conpherence-header-pane',
-                'sigil' => 'conpherence-messages',
+                'sigil' => 'conpherence-header',
               ),
               ''),
             phutil_tag(
               'div',
               array(
-                'id' => 'conpherence-form'
+                'class' => 'conpherence-widget-pane',
+                'id' => 'conpherence-widget-pane'
               ),
-              '')
+              ''),
-          ))
+            javelin_tag(
+              'div',
+              array(
+                'class' => 'conpherence-message-pane',
+                'id' => 'conpherence-message-pane'
+              ),
+              array(
+                javelin_tag(
+                  'div',
+                  array(
+                    'class' => 'conpherence-messages',
+                    'id' => 'conpherence-messages',
+                    'sigil' => 'conpherence-messages',
+                  ),
+                  ''),
+                phutil_tag(
+                  'div',
+                  array(
+                    'id' => 'conpherence-form'
+                  ),
+                  '')
+              )),
+          )),
       ));
   }
 

src/applications/conpherence/view/ConpherenceThreadListView.php

@@ -0,0 +1,112 @@
+<?php
+
+final class ConpherenceThreadListView extends AphrontView {
+
+  private $baseURI;
+  private $unreadThreads;
+  private $readThreads;
+
+  public function setBaseURI($base_uri) {
+    $this->baseURI = $base_uri;
+    return $this;
+  }
+
+  public function setUnreadThreads(array $unread_threads) {
+    assert_instances_of($unread_threads, 'ConpherenceThread');
+    $this->unreadThreads = $unread_threads;
+    return $this;
+  }
+
+  public function setReadThreads(array $read_threads) {
+    assert_instances_of($read_threads, 'ConpherenceThread');
+    $this->readThreads = $read_threads;
+    return $this;
+  }
+
+  public function render() {
+    require_celerity_resource('conpherence-menu-css');
+
+    $menu = id(new PhabricatorMenuView())
+      ->addClass('conpherence-menu')
+      ->setID('conpherence-menu');
+
+    $menu->addMenuItem(
+      id(new PhabricatorMenuItemView())
+        ->setName(pht('New Conversation'))
+        ->setWorkflow(true)
+        ->setKey('new')
+        ->setHref($this->baseURI.'new/')
+        ->setType(PhabricatorMenuItemView::TYPE_BUTTON));
+
+    $menu->newLabel(pht('Unread'));
+    $this->addThreadsToMenu($menu, $this->unreadThreads, $read = false);
+    $menu->newLabel(pht('Read'));
+    $this->addThreadsToMenu($menu, $this->readThreads, $read = true);
+
+    return $menu;
+  }
+
+  private function renderThread(ConpherenceThread $thread) {
+    $user = $this->getUser();
+
+    $uri = $this->baseURI.$thread->getID().'/';
+    $data = $thread->getDisplayData($user, null);
+    $title = $data['title'];
+    $subtitle = $data['subtitle'];
+    $unread_count = $data['unread_count'];
+    $epoch = $data['epoch'];
+    $image = $data['image'];
+    $snippet = $data['snippet'];
+
+    $item = id(new ConpherenceMenuItemView())
+      ->setUser($user)
+      ->setTitle($title)
+      ->setSubtitle($subtitle)
+      ->setHref($uri)
+      ->setEpoch($epoch)
+      ->setImageURI($image)
+      ->setMessageText($snippet)
+      ->setUnreadCount($unread_count)
+      ->setID($thread->getPHID().'-nav-item')
+      ->addSigil('conpherence-menu-click')
+      ->setMetadata(
+        array(
+          'id' => $thread->getID(),
+          ));
+
+    return id(new PhabricatorMenuItemView())
+      ->setType(PhabricatorMenuItemView::TYPE_CUSTOM)
+      ->setName($item);
+  }
+
+  private function addThreadsToMenu(
+    PhabricatorMenuView $menu,
+    array $conpherences,
+    $read = false) {
+
+    foreach ($conpherences as $conpherence) {
+      $item = $this->renderThread($conpherence);
+      $menu->addMenuItem($item);
+    }
+
+    if (empty($conpherences) || $read) {
+      $menu->addMenuItem($this->getNoConpherencesBlock());
+    }
+
+    return $menu;
+  }
+
+  private function getNoConpherencesBlock() {
+    $message = phutil_tag(
+      'div',
+      array(
+        'class' => 'no-conpherences-menu-item'
+      ),
+      pht('No more conpherences.'));
+
+    return id(new PhabricatorMenuItemView())
+      ->setType(PhabricatorMenuItemView::TYPE_CUSTOM)
+      ->setName($message);
+  }
+
+}

webroot/rsrc/css/application/conpherence/header-pane.css

@@ -2,10 +2,6 @@
  * @provides conpherence-header-pane-css
  */
 
-#conpherence-main-pane {
-  position: relative;
-}
-
 .conpherence-header-pane {
   border-bottom: 1px solid #ccc;
   height: 80px;

webroot/rsrc/css/application/conpherence/menu.css

@@ -2,36 +2,31 @@
  * @provides conpherence-menu-css
  */
 
-.no-conpherences-menu-item {
+.conpherence-layout {
-  color: #a1a5a9;
+  position: fixed;
-  border-top: solid 1px #3B3D3E;
+  bottom: 0;
-  padding: 20px 0;
+  left: 0;
-  margin: 0px auto;
+  right: 0;
-  width: 300px;
-  text-align: center;
-}
-
-.conpherence-menu .phabricator-nav-column-background,
-.conpherence-menu .phabricator-nav-local {
-  width: 300px;
-}
-.conpherence-menu .phabricator-nav-local {
   top: 44px;
-  bottom: 0px;
 }
 
-.conpherence-menu .phabricator-nav-drag {
+.conpherence-menu-pane {
-  left: 300px;
+  width: 300px;
-}
+  position: absolute;
-
-.device-desktop .conpherence-menu .phabricator-nav-content {
-  margin-left: 300px !important;
-}
-
-.conpherence-menu .phabricator-menu-view {
   overflow-x: hidden;
   overflow-y: auto;
-  margin-bottom: 0;
+  top: 0;
+  bottom: 0;
+}
+
+.conpherence-content-pane {
+  margin-left: 300px;
+  position: relative;
+}
+
+div.conpherence-layout .phabricator-nav-column-background {
+  display: block;
+  width: 300px;
 }
 
 .conpherence-menu .conpherence-menu-item-view {
@@ -122,3 +117,12 @@
   color: white;
   font-size: 12px;
 }
+
+.no-conpherences-menu-item {
+  color: #a1a5a9;
+  border-top: solid 1px #3B3D3E;
+  padding: 20px 0;
+  margin: 0px auto;
+  width: 300px;
+  text-align: center;
+}

webroot/rsrc/css/core/z-index.css

@@ -30,6 +30,10 @@
   z-index: 4;
 }
 
+.conpherence-menu-pane {
+  z-index: 4;
+}
+
 .phabricator-nav-drag {
   z-index: 4;
 }