From f75248de6f4b2eea33ab4f5a8389221fece25b15 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 31 Mar 2015 12:45:32 -0700 Subject: [PATCH] Conpherence - refine menu interactions with respect to "Show More" Summary: Ref T7566. Prior to this diff, we had a broken mess in the "Messages" section. Now, "Messages" behave like rooms in that whatever is loaded at page load time is at the top of the list. Additionally, refine "show more" behavior such that it simply shows the next X, but if there exists X + 1 then we have another "show more" that kicks you to application search. Theoretically, there are still corner cases where users are in a ton of rooms or a ton of messages respectively, but this feels pretty good. Consolidates title rendering code so we always render the list of participants and no more "No Title". Also remove the policy icons for messages consistently, helping to differentiate them from rooms at a glance. Test Plan: clicked around in conpherence main - looked good. tried "show more" and it worked! played around in durable column and things seemed reasonable there too. Reviewers: chad, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7566 Differential Revision: https://secure.phabricator.com/D12222 --- resources/celerity/map.php | 32 ++-- .../controller/ConpherenceController.php | 15 +- .../controller/ConpherenceListController.php | 167 +++--------------- .../ConpherenceUpdateController.php | 5 +- .../controller/ConpherenceViewController.php | 4 +- .../PhabricatorConpherenceThreadPHIDType.php | 10 +- .../query/ConpherenceThreadSearchEngine.php | 4 + .../conpherence/storage/ConpherenceThread.php | 10 +- .../view/ConpherenceDurableColumnView.php | 19 +- .../view/ConpherenceThreadListView.php | 167 +++++++++--------- .../rsrc/css/application/conpherence/menu.css | 4 + .../application/conpherence/behavior-menu.js | 75 ++------ 12 files changed, 170 insertions(+), 342 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index deba170794..8f1dfc6060 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -45,7 +45,7 @@ return array( 'rsrc/css/application/config/setup-issue.css' => '22270af2', 'rsrc/css/application/config/unhandled-exception.css' => '37d4f9a2', 'rsrc/css/application/conpherence/durable-column.css' => 'caa12d4a', - 'rsrc/css/application/conpherence/menu.css' => 'beef0920', + 'rsrc/css/application/conpherence/menu.css' => '7c900089', 'rsrc/css/application/conpherence/message-pane.css' => '44154798', 'rsrc/css/application/conpherence/notification.css' => '04a6e10a', 'rsrc/css/application/conpherence/update.css' => '1099a660', @@ -355,7 +355,7 @@ return array( 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'bb928342', 'rsrc/js/application/conpherence/behavior-durable-column.js' => 'c81c2bba', - 'rsrc/js/application/conpherence/behavior-menu.js' => 'de5579b4', + 'rsrc/js/application/conpherence/behavior-menu.js' => 'cda6de80', 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '1ec93bcf', @@ -515,7 +515,7 @@ return array( 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', 'conpherence-durable-column-view' => 'caa12d4a', - 'conpherence-menu-css' => 'beef0920', + 'conpherence-menu-css' => '7c900089', 'conpherence-message-pane-css' => '44154798', 'conpherence-notification-css' => '04a6e10a', 'conpherence-thread-manager' => 'bb928342', @@ -557,7 +557,7 @@ return array( 'javelin-behavior-boards-dropdown' => '0ec56e1d', 'javelin-behavior-choose-control' => '6153c708', 'javelin-behavior-config-reorder-fields' => '14a827de', - 'javelin-behavior-conpherence-menu' => 'de5579b4', + 'javelin-behavior-conpherence-menu' => 'cda6de80', 'javelin-behavior-conpherence-pontificate' => '21ba5861', 'javelin-behavior-conpherence-widget-pane' => '1ec93bcf', 'javelin-behavior-countdown-timer' => 'e4cc26b3', @@ -1755,6 +1755,18 @@ return array( 'javelin-stratcom', 'phabricator-phtize', ), + 'cda6de80' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-behavior-device', + 'javelin-history', + 'javelin-vector', + 'phabricator-shaped-request', + 'conpherence-thread-manager', + ), 'd19198c8' => array( 'javelin-install', 'javelin-dom', @@ -1807,18 +1819,6 @@ return array( 'javelin-typeahead-ondemand-source', 'javelin-dom', ), - 'de5579b4' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-behavior-device', - 'javelin-history', - 'javelin-vector', - 'phabricator-shaped-request', - 'conpherence-thread-manager', - ), 'e10f8e18' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index 6c7a331603..e78b5597dc 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -80,8 +80,8 @@ abstract class ConpherenceController extends PhabricatorController { assert_instances_of($policy_objects, 'PhabricatorPolicy'); $crumbs = $this->buildApplicationCrumbs(); - $title = $this->getConpherenceTitle($conpherence); - if ($conpherence->getID()) { + $data = $conpherence->getDisplayData($this->getViewer()); + if ($conpherence->getID() && $conpherence->getIsRoom()) { $icon = $conpherence->getPolicyIconName($policy_objects); } else { $icon = null; @@ -89,7 +89,7 @@ abstract class ConpherenceController extends PhabricatorController { $crumbs->addCrumb( id(new PHUICrumbView()) ->setIcon($icon) - ->setName($title) + ->setName($data['title']) ->setHref($this->getApplicationURI('update/'.$conpherence->getID().'/')) ->setWorkflow(true)); @@ -106,13 +106,4 @@ abstract class ConpherenceController extends PhabricatorController { )); } - protected function getConpherenceTitle(ConpherenceThread $conpherence) { - if ($conpherence->getTitle()) { - $title = $conpherence->getTitle(); - } else { - $title = pht('[No Title]'); - } - return $title; - } - } diff --git a/src/applications/conpherence/controller/ConpherenceListController.php b/src/applications/conpherence/controller/ConpherenceListController.php index de79d1c720..38190367a9 100644 --- a/src/applications/conpherence/controller/ConpherenceListController.php +++ b/src/applications/conpherence/controller/ConpherenceListController.php @@ -4,14 +4,12 @@ final class ConpherenceListController extends ConpherenceController { const SELECTED_MODE = 'selected'; const UNSELECTED_MODE = 'unselected'; - const PAGING_MODE = 'paging'; /** - * Three main modes of operation... + * Two main modes of operation... * * 1 - /conpherence/ - UNSELECTED_MODE * 2 - /conpherence// - SELECTED_MODE - * 3 - /conpherence/?direction='up'&... - PAGING_MODE * * UNSELECTED_MODE is not an Ajax request while the other two are Ajax * requests. @@ -21,11 +19,7 @@ final class ConpherenceListController extends ConpherenceController { $mode = self::UNSELECTED_MODE; if ($request->isAjax()) { - if ($request->getStr('direction')) { - $mode = self::PAGING_MODE; - } else { - $mode = self::SELECTED_MODE; - } + $mode = self::SELECTED_MODE; } return $mode; @@ -36,9 +30,7 @@ final class ConpherenceListController extends ConpherenceController { $title = pht('Conpherence'); $conpherence = null; - $scroll_up_participant = $this->getEmptyParticipant(); - $scroll_down_participant = $this->getEmptyParticipant(); - $too_many = ConpherenceParticipantQuery::LIMIT + 1; + $limit = ConpherenceThreadListView::SEE_MORE_LIMIT * 5; $all_participation = array(); $mode = $this->determineMode(); @@ -56,60 +48,24 @@ final class ConpherenceListController extends ConpherenceController { $title = $conpherence->getTitle(); } $cursor = $conpherence->getParticipantIfExists($user->getPHID()); - if (!$cursor || $conpherence->getIsRoom()) { - $data = $this->loadDefaultParticipation($too_many); - $all_participation = $data['all_participation']; - $scroll_down_participant = $data['scroll_down_participant']; - $menu_participation = $this->getEmptyParticipant() + $data = $this->loadDefaultParticipation($limit); + $all_participation = $data['all_participation']; + if (!$cursor) { + $menu_participation = id(new ConpherenceParticipant()) + ->makeEphemeral() ->setConpherencePHID($conpherence->getPHID()) ->setParticipantPHID($user->getPHID()); - $all_participation = - array($conpherence->getPHID() => $menu_participation) + - $all_participation; - } else { - $data = $this->loadParticipationWithMidCursor($cursor); - $all_participation = $data['all_participation']; - $scroll_up_participant = $data['scroll_up_participant']; - $scroll_down_participant = $data['scroll_down_participant']; + $menu_participation = $cursor; } - break; - case self::PAGING_MODE: - $direction = $request->getStr('direction'); - $id = $request->getInt('participant_id'); - $date_touched = $request->getInt('date_touched'); - $conpherence_phid = $request->getStr('conpherence_phid'); - if ($direction == 'up') { - $order = ConpherenceParticipantQuery::ORDER_NEWER; - } else { - $order = ConpherenceParticipantQuery::ORDER_OLDER; - } - $scroller_participant = id(new ConpherenceParticipant()) - ->makeEphemeral() - ->setID($id) - ->setDateTouched($date_touched) - ->setConpherencePHID($conpherence_phid); - $participation = id(new ConpherenceParticipantQuery()) - ->withParticipantPHIDs(array($user->getPHID())) - ->withParticipantCursor($scroller_participant) - ->setOrder($order) - ->setLimit($too_many) - ->execute(); - if (count($participation) == $too_many) { - if ($direction == 'up') { - $node = $scroll_up_participant = reset($participation); - } else { - $node = $scroll_down_participant = end($participation); - } - unset($participation[$node->getConpherencePHID()]); - } - $all_participation = $participation; + $all_participation = + array($conpherence->getPHID() => $menu_participation) + + $all_participation; break; case self::UNSELECTED_MODE: default: - $data = $this->loadDefaultParticipation($too_many); + $data = $this->loadDefaultParticipation($limit); $all_participation = $data['all_participation']; - $scroll_down_participant = $data['scroll_down_participant']; break; } @@ -119,23 +75,12 @@ final class ConpherenceListController extends ConpherenceController { $thread_view = id(new ConpherenceThreadListView()) ->setUser($user) ->setBaseURI($this->getApplicationURI()) - ->setThreads($threads) - ->setScrollUpParticipant($scroll_up_participant) - ->setScrollDownParticipant($scroll_down_participant); + ->setThreads($threads); switch ($mode) { case self::SELECTED_MODE: $response = id(new AphrontAjaxResponse())->setContent($thread_view); break; - case self::PAGING_MODE: - $thread_html = $thread_view->renderThreadsHTML(); - $phids = array_keys($participation); - $content = array( - 'html' => $thread_html, - 'phids' => $phids, - ); - $response = id(new AphrontAjaxResponse())->setContent($content); - break; case self::UNSELECTED_MODE: default: $layout = id(new ConpherenceLayoutView()) @@ -153,11 +98,11 @@ final class ConpherenceListController extends ConpherenceController { $policy_objects)); $layout->setThread($conpherence); } else { + $thread = ConpherenceThread::initializeNewThread($user); + $thread->attachHandles(array()); + $thread->makeEphemeral(); $layout->setHeader( - $this->buildHeaderPaneContent( - id(new ConpherenceThread()) - ->makeEphemeral(), - array())); + $this->buildHeaderPaneContent($thread, array())); } $response = $this->buildApplicationPage( $layout, @@ -171,81 +116,16 @@ final class ConpherenceListController extends ConpherenceController { } - private function loadDefaultParticipation($too_many) { + private function loadDefaultParticipation($limit) { $viewer = $this->getRequest()->getUser(); - $scroll_down_participant = $this->getEmptyParticipant(); - $all_participation = id(new ConpherenceParticipantQuery()) ->withParticipantPHIDs(array($viewer->getPHID())) - ->setLimit($too_many) + ->setLimit($limit) ->execute(); - if (count($all_participation) == $too_many) { - $node = end($all_participation); - unset($all_participation[$node->getConpherencePHID()]); - $scroll_down_participant = $node; - } return array( - 'all_participation' => $all_participation, - 'scroll_down_participant' => $scroll_down_participant,); - } - - /** - * Handles the curious case when we are visiting a conpherence directly - * by issuing two separate queries. Otherwise, additional conpherences - * are fetched asynchronously. Note these can be earlier or later - * (up or down), depending on what conpherence was selected on initial - * load. - */ - private function loadParticipationWithMidCursor( - ConpherenceParticipant $cursor) { - - $user = $this->getRequest()->getUser(); - - $scroll_up_participant = $this->getEmptyParticipant(); - $scroll_down_participant = $this->getEmptyParticipant(); - - // Note this is a bit dodgy since there may be less than this - // amount in either the up or down direction, thus having us fail - // to fetch LIMIT in total. Whatevs for now and re-visit if we're - // fine-tuning this loading process. - $too_many = ceil(ConpherenceParticipantQuery::LIMIT / 2) + 1; - $participant_query = id(new ConpherenceParticipantQuery()) - ->withParticipantPHIDs(array($user->getPHID())) - ->setLimit($too_many); - $current_selection_epoch = $cursor->getDateTouched(); - $set_one = $participant_query - ->withParticipantCursor($cursor) - ->setOrder(ConpherenceParticipantQuery::ORDER_NEWER) - ->execute(); - - if (count($set_one) == $too_many) { - $node = reset($set_one); - unset($set_one[$node->getConpherencePHID()]); - $scroll_up_participant = $node; - } - - $set_two = $participant_query - ->withParticipantCursor($cursor) - ->setOrder(ConpherenceParticipantQuery::ORDER_OLDER) - ->execute(); - - if (count($set_two) == $too_many) { - $node = end($set_two); - unset($set_two[$node->getConpherencePHID()]); - $scroll_down_participant = $node; - } - - $participation = array_merge( - $set_one, - $set_two); - - return array( - 'scroll_up_participant' => $scroll_up_participant, - 'scroll_down_participant' => $scroll_down_participant, - 'all_participation' => $participation, - ); + 'all_participation' => $all_participation,); } private function loadConpherenceThreadData($participation) { @@ -266,9 +146,4 @@ final class ConpherenceListController extends ConpherenceController { return $conpherences; } - private function getEmptyParticipant() { - return id(new ConpherenceParticipant()) - ->makeEphemeral(); - } - } diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 132e590c99..3b67b6c1df 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -31,6 +31,7 @@ final class ConpherenceUpdateController ->setViewer($user) ->withIDs(array($conpherence_id)) ->needFilePHIDs(true) + ->needParticipantCache(true) ->requireCapabilities($needed_capabilities) ->executeOne(); @@ -409,10 +410,10 @@ final class ConpherenceUpdateController if ($people_widget) { $people_html = hsprintf('%s', $people_widget->render()); } - $title = $this->getConpherenceTitle($conpherence); + $data = $conpherence->getDisplayData($user); $content = array( 'transactions' => hsprintf('%s', $rendered_transactions), - 'conpherence_title' => (string) $title, + 'conpherence_title' => (string) $data['title'], 'latest_transaction_id' => $new_latest_transaction_id, 'nav_item' => hsprintf('%s', $nav_item), 'conpherence_phid' => $conpherence->getPHID(), diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index efbc72531a..04b92d176a 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -60,8 +60,8 @@ final class ConpherenceViewController extends ); } - $title = $this->getConpherenceTitle($conpherence); - $content['title'] = $title; + $d_data = $conpherence->getDisplayData($user); + $content['title'] = $title = $d_data['title']; if ($request->isAjax()) { $content['threadID'] = $conpherence->getID(); diff --git a/src/applications/conpherence/phid/PhabricatorConpherenceThreadPHIDType.php b/src/applications/conpherence/phid/PhabricatorConpherenceThreadPHIDType.php index 0577956bbe..e85c4467c5 100644 --- a/src/applications/conpherence/phid/PhabricatorConpherenceThreadPHIDType.php +++ b/src/applications/conpherence/phid/PhabricatorConpherenceThreadPHIDType.php @@ -17,6 +17,7 @@ final class PhabricatorConpherenceThreadPHIDType extends PhabricatorPHIDType { array $phids) { return id(new ConpherenceThreadQuery()) + ->needParticipantCache(true) ->withPHIDs($phids); } @@ -27,12 +28,9 @@ final class PhabricatorConpherenceThreadPHIDType extends PhabricatorPHIDType { foreach ($handles as $phid => $handle) { $thread = $objects[$phid]; - $name = $thread->getTitle(); - if (!strlen($name)) { - $name = pht('[No Title]'); - } - $handle->setName($name); - $handle->setFullName($name); + $data = $thread->getDisplayData($query->getViewer()); + $handle->setName($data['title']); + $handle->setFullName($data['title']); $handle->setURI('/conpherence/'.$thread->getID().'/'); } } diff --git a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php index b137850e46..01a66dfd4b 100644 --- a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php +++ b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php @@ -80,6 +80,7 @@ final class ConpherenceThreadSearchEngine if ($this->requireViewer()->isLoggedIn()) { $names['participant'] = pht('Joined Rooms'); + $names['messages'] = pht('All Messages'); } } @@ -98,6 +99,9 @@ final class ConpherenceThreadSearchEngine return $query->setParameter( 'participantPHIDs', array($this->requireViewer()->getPHID())); + case 'messages': + $this->setIsRooms(false); + return $query; } return parent::buildSavedQueryFromBuiltin($query_key); diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index 1fe2eb1a63..148f2206ac 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -174,11 +174,10 @@ final class ConpherenceThread extends ConpherenceDAO } $title = $js_title = $this->getTitle(); - if (!$title) { - $title = $lucky_handle->getName(); - $js_title = pht('[No Title]'); + $img_src = null; + if ($lucky_handle) { + $img_src = $lucky_handle->getImageURI(); } - $img_src = $lucky_handle->getImageURI(); $count = 0; $final = false; @@ -200,6 +199,9 @@ final class ConpherenceThread extends ConpherenceDAO $count++; $final = $count == 3; } + if (!$title) { + $title = $js_title = $subtitle; + } $user_participation = $this->getParticipantIfExists($user->getPHID()); if ($user_participation) { diff --git a/src/applications/conpherence/view/ConpherenceDurableColumnView.php b/src/applications/conpherence/view/ConpherenceDurableColumnView.php index ed88ebb8be..7fd765ad0b 100644 --- a/src/applications/conpherence/view/ConpherenceDurableColumnView.php +++ b/src/applications/conpherence/view/ConpherenceDurableColumnView.php @@ -213,10 +213,14 @@ final class ConpherenceDurableColumnView extends AphrontTagView { assert_instances_of($policy_objects, 'PhabricatorPolicy'); - $icon = $conpherence->getPolicyIconName($policy_objects); - return id(new PHUIIconView()) - ->addClass('mmr') - ->setIconFont($icon); + $icon = null; + if ($conpherence->getIsRoom()) { + $icon = $conpherence->getPolicyIconName($policy_objects); + $icon = id(new PHUIIconView()) + ->addClass('mmr') + ->setIconFont($icon); + } + return $icon; } private function buildIconBar() { @@ -346,16 +350,13 @@ final class ConpherenceDurableColumnView extends AphrontTagView { ->addClass('phabricator-dark-menu') ->addClass('phabricator-application-menu'); - $title = $conpherence->getTitle(); - if (!$title) { - $title = pht('[No Title]'); - } + $data = $conpherence->getDisplayData($this->getUser()); $header = phutil_tag( 'span', array(), array( $this->getPolicyIcon($conpherence, $this->getPolicyObjects()), - $title, + $data['title'], )); } diff --git a/src/applications/conpherence/view/ConpherenceThreadListView.php b/src/applications/conpherence/view/ConpherenceThreadListView.php index e0008fa3c4..65c9bddffa 100644 --- a/src/applications/conpherence/view/ConpherenceThreadListView.php +++ b/src/applications/conpherence/view/ConpherenceThreadListView.php @@ -2,10 +2,10 @@ final class ConpherenceThreadListView extends AphrontView { + const SEE_MORE_LIMIT = 5; + private $baseURI; private $threads; - private $scrollUpParticipant; - private $scrollDownParticipant; public function setThreads(array $threads) { assert_instances_of($threads, 'ConpherenceThread'); @@ -13,18 +13,6 @@ final class ConpherenceThreadListView extends AphrontView { return $this; } - public function setScrollUpParticipant( - ConpherenceParticipant $participant) { - $this->scrollUpParticipant = $participant; - return $this; - } - - public function setScrollDownParticipant( - ConpherenceParticipant $participant) { - $this->scrollDownParticipant = $participant; - return $this; - } - public function setBaseURI($base_uri) { $this->baseURI = $base_uri; return $this; @@ -35,7 +23,6 @@ final class ConpherenceThreadListView extends AphrontView { $grouped = mgroup($this->threads, 'getIsRoom'); $rooms = idx($grouped, true, array()); - $rooms = array_slice($rooms, 0, 5); $policies = array(); foreach ($rooms as $room) { @@ -55,7 +42,7 @@ final class ConpherenceThreadListView extends AphrontView { $this->addRoomsToMenu($menu, $rooms, $policy_objects); $messages = idx($grouped, false, array()); - $this->addThreadsToMenu($menu, $messages); + $this->addMessagesToMenu($menu, $messages); return $menu; } @@ -71,22 +58,10 @@ final class ConpherenceThreadListView extends AphrontView { public function renderThreadsHTML() { $thread_html = array(); - if ($this->scrollUpParticipant->getID()) { - $thread_html[] = $this->getScrollMenuItem( - $this->scrollUpParticipant, - 'up'); - } - foreach ($this->threads as $thread) { $thread_html[] = $this->renderSingleThread($thread); } - if ($this->scrollDownParticipant->getID()) { - $thread_html[] = $this->getScrollMenuItem( - $this->scrollDownParticipant, - 'down'); - } - return phutil_implode_html('', $thread_html); } @@ -106,13 +81,17 @@ final class ConpherenceThreadListView extends AphrontView { $uri = $this->baseURI.$thread->getID().'/'; $data = $thread->getDisplayData($user); + $icon = null; + if ($thread->getIsRoom()) { + $icon = id(new PHUIIconView()) + ->addClass('msr') + ->setIconFont($thread->getPolicyIconName($policy_objects)); + } $title = phutil_tag( 'span', array(), array( - id(new PHUIIconView()) - ->addClass('msr') - ->setIconFont($thread->getPolicyIconName($policy_objects)), + $icon, $data['title'], )); $subtitle = $data['subtitle']; @@ -141,7 +120,7 @@ final class ConpherenceThreadListView extends AphrontView { private function addRoomsToMenu( PHUIListView $menu, - array $conpherences, + array $rooms, array $policy_objects) { $header = $this->renderMenuItemHeader( @@ -154,7 +133,7 @@ final class ConpherenceThreadListView extends AphrontView { ->setText(pht('Search'))); $menu->addMenuItem($header); - if (empty($conpherences)) { + if (empty($rooms)) { $join_item = id(new PHUIListItemView()) ->setType(PHUIListItemView::TYPE_LINK) ->setHref('/conpherence/search/') @@ -171,46 +150,95 @@ final class ConpherenceThreadListView extends AphrontView { return $menu; } - foreach ($conpherences as $conpherence) { - $item = $this->renderThreadItem($conpherence, $policy_objects); - $menu->addMenuItem($item); + $this->addThreadsToMenu($menu, $rooms, $policy_objects); + return $menu; + } + + private function addMessagesToMenu( + PHUIListView $menu, + array $conpherences) { + + $header = $this->renderMenuItemHeader( + pht('Messages'), + 'conpherence-message-list-header'); + $menu->addMenuItem($header); + + if (empty($conpherences)) { + $menu->addMenuItem($this->getNoConpherencesMenuItem()); + return $menu; } - $more_item = id(new PHUIListItemView()) - ->setType(PHUIListItemView::TYPE_LINK) - ->setHref('/conpherence/search/query/participant/') - ->setName(pht('See More')); - $menu->addMenuItem($more_item); + $this->addThreadsToMenu($menu, $conpherences, array()); return $menu; } - private function addThreadsToMenu( PHUIListView $menu, - array $conpherences) { + array $threads, + array $policy_objects) { - if ($this->scrollUpParticipant->getID()) { - $item = $this->getScrollMenuItem($this->scrollUpParticipant, 'up'); - $menu->addMenuItem($item); + // If we have self::SEE_MORE_LIMIT or less, we can just render + // all the threads at once. Otherwise, we render a "See more" + // UI element, which toggles a show / hide on the remaining rooms + $show_threads = $threads; + $more_threads = array(); + if (count($threads) > self::SEE_MORE_LIMIT) { + $show_threads = array_slice($threads, 0, self::SEE_MORE_LIMIT); + $more_threads = array_slice($threads, self::SEE_MORE_LIMIT); } - $header = $this->renderMenuItemHeader( - pht('Messages'), 'conpherence-message-list-header'); - $menu->addMenuItem($header); - - foreach ($conpherences as $conpherence) { - $item = $this->renderThreadItem($conpherence); + $is_room = false; + foreach ($show_threads as $thread) { + $item = $this->renderThreadItem($thread, $policy_objects); $menu->addMenuItem($item); + $is_room = $thread->getIsRoom(); } - if (empty($conpherences)) { - $menu->addMenuItem($this->getNoConpherencesMenuItem()); - } + if ($more_threads) { + if ($is_room) { + $search_uri = '/conpherence/search/query/participant/'; + $sigil = 'more-room'; + } else { + $search_uri = '/conpherence/search/query/messages/'; + $sigil = 'more-message'; + } - if ($this->scrollDownParticipant->getID()) { - $item = $this->getScrollMenuItem($this->scrollDownParticipant, 'down'); - $menu->addMenuItem($item); + $more_item = id(new PHUIListItemView()) + ->setType(PHUIListItemView::TYPE_LINK) + ->setHref($search_uri) + ->addSigil('conpherence-menu-see-more') + ->setMetadata(array('moreSigil' => $sigil)) + ->setName(pht('See More')); + $menu->addMenuItem($more_item); + $show_more_threads = $more_threads; + $even_more_threads = array(); + if (count($more_threads) > self::SEE_MORE_LIMIT) { + $show_more_threads = array_slice( + $more_threads, + 0, + self::SEE_MORE_LIMIT); + $even_more_threads = array_slice( + $more_threads, + self::SEE_MORE_LIMIT); + } + foreach ($show_more_threads as $thread) { + $item = $this->renderThreadItem($thread, $policy_objects) + ->addSigil($sigil) + ->addClass('hidden'); + $menu->addMenuItem($item); + } + + if ($even_more_threads) { + // kick them to application search here + $even_more_item = id(new PHUIListItemView()) + ->setType(PHUIListItemView::TYPE_LINK) + ->setHref($search_uri) + ->addSigil($sigil) + ->addClass('hidden') + ->setName(pht('See More')); + $menu->addMenuItem($even_more_item); + } } return $menu; @@ -224,29 +252,6 @@ final class ConpherenceThreadListView extends AphrontView { return $item; } - public function getScrollMenuItem( - ConpherenceParticipant $participant, - $direction) { - - if ($direction == 'up') { - $name = pht('Load Newer Threads'); - } else { - $name = pht('Load Older Threads'); - } - $item = id(new PHUIListItemView()) - ->addSigil('conpherence-menu-scroller') - ->setName($name) - ->setHref($this->baseURI) - ->setType(PHUIListItemView::TYPE_BUTTON) - ->setMetadata(array( - 'participant_id' => $participant->getID(), - 'conpherence_phid' => $participant->getConpherencePHID(), - 'date_touched' => $participant->getDateTouched(), - 'direction' => $direction, - )); - return $item; - } - private function getNoMessagesMenuItem() { $message = phutil_tag( 'div', diff --git a/webroot/rsrc/css/application/conpherence/menu.css b/webroot/rsrc/css/application/conpherence/menu.css index 5b38944fd5..ad402c5eb2 100644 --- a/webroot/rsrc/css/application/conpherence/menu.css +++ b/webroot/rsrc/css/application/conpherence/menu.css @@ -43,6 +43,10 @@ border-right: 1px solid {$hovergrey} } +.conpherence-menu-pane .phui-list-item-view.hidden { + display: none; +} + .conpherence-menu-pane.phabricator-side-menu .phui-list-item-type-label { padding: 10px 0 9px 8px; } diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 88e23bd672..62d30abec1 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -478,8 +478,6 @@ JX.behavior('conpherence-menu', function(config) { config.selectedID && selectThreadByID(config.selectedID); - _thread.node.scrollIntoView(); - markThreadsLoading(false); } @@ -504,70 +502,19 @@ JX.behavior('conpherence-menu', function(config) { } } - var handleThreadScrollers = function (e) { - e.kill(); - - var data = e.getNodeData('conpherence-menu-scroller'); - var scroller = e.getNode('conpherence-menu-scroller'); - JX.DOM.alterClass(scroller, 'loading', true); - JX.DOM.setContent(scroller.firstChild, 'Loading...'); - new JX.Workflow(scroller.href, data) - .setHandler( - JX.bind(null, threadScrollerResponse, scroller, data.direction)) - .start(); - }; - - var threadScrollerResponse = function (scroller, direction, r) { - var html = JX.$H(r.html); - - var thread_phids = r.phids; - var reselect_id = null; - // remove any threads that are in the list that we just got back - // in the result set; things have changed and they'll be in the - // right place soon - for (var ii = 0; ii < thread_phids.length; ii++) { - try { - var node_id = thread_phids[ii] + '-nav-item'; - var node = JX.$(node_id); - var node_data = JX.Stratcom.getData(node); - if (node_data.id == _thread.selected) { - reselect_id = node_id; - } - JX.DOM.remove(node); - } catch (ex) { - // ignore , just haven't seen this thread yet - } - } - - var root = JX.DOM.find(document, 'div', 'conpherence-layout'); - var menu_root = JX.DOM.find(root, 'div', 'conpherence-menu-pane'); - var scroll_y = 0; - // we have to do some hyjinx in the up case to make the menu scroll to - // where it should - if (direction == 'up') { - var style = { - position: 'absolute', - left: '-10000px' - }; - var test_size = JX.$N('div', {style: style}, html); - document.body.appendChild(test_size); - var html_size = JX.Vector.getDim(test_size); - JX.DOM.remove(test_size); - scroll_y = html_size.y; - } - JX.DOM.replace(scroller, html); - menu_root.scrollTop += scroll_y; - - if (reselect_id) { - selectThreadByID(reselect_id); - } - }; - JX.Stratcom.listen( ['click'], - 'conpherence-menu-scroller', - handleThreadScrollers - ); + 'conpherence-menu-see-more', + function (e) { + e.kill(); + var sigil = e.getNodeData('conpherence-menu-see-more').moreSigil; + var root = JX.$('conpherence-menu-pane'); + var more = JX.DOM.scry(root, 'li', sigil); + for (var i = 0; i < more.length; i++) { + JX.DOM.alterClass(more[i], 'hidden', false); + } + JX.DOM.hide(e.getNode('conpherence-menu-see-more')); + }); JX.Stratcom.listen( ['keydown'],