From 1fd063277b0e2641e6d8b45a34750b699f1c5264 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Apr 2013 12:43:21 -0700 Subject: [PATCH] Move first-thread selection to the client in Conpherence Summary: Ref T2421. For mobile, we don't want to select the first thread, since we'll just show a list of threads. Move this behavior to the client and do it when the page is shown on a desktop (or the view is changed to a desktop). Test Plan: Viewed `/conpherence/`, saw first thread selected. Switched threads. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2421 Differential Revision: https://secure.phabricator.com/D5502 --- src/__celerity_resource_map__.php | 7 +- .../controller/ConpherenceController.php | 37 +--- .../controller/ConpherenceListController.php | 5 +- .../application/conpherence/behavior-menu.js | 177 ++++++++++-------- 4 files changed, 107 insertions(+), 119 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 9f20dac0bf..5ce13ad310 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1253,7 +1253,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-conpherence-menu' => array( - 'uri' => '/res/ec121ad7/rsrc/js/application/conpherence/behavior-menu.js', + 'uri' => '/res/2377a63e/rsrc/js/application/conpherence/behavior-menu.js', 'type' => 'js', 'requires' => array( @@ -1261,9 +1261,8 @@ celerity_register_resource_map(array( 1 => 'javelin-dom', 2 => 'javelin-request', 3 => 'javelin-stratcom', - 4 => 'javelin-uri', - 5 => 'javelin-util', - 6 => 'javelin-workflow', + 4 => 'javelin-workflow', + 5 => 'javelin-behavior-device', ), 'disk' => '/rsrc/js/application/conpherence/behavior-menu.js', ), diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index 3da95bd2e1..c697eb5af1 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -5,7 +5,6 @@ */ abstract class ConpherenceController extends PhabricatorController { private $conpherences; - private $selectedConpherencePHID; private $readConpherences; private $unreadConpherences; @@ -27,14 +26,6 @@ abstract class ConpherenceController extends PhabricatorController { return $this->readConpherences; } - public function setSelectedConpherencePHID($phid) { - $this->selectedConpherencePHID = $phid; - return $this; - } - public function getSelectedConpherencePHID() { - return $this->selectedConpherencePHID; - } - /** * Try for a full set of unread conpherences, and if we fail * load read conpherences. Additional conpherences in either category @@ -86,10 +77,6 @@ abstract class ConpherenceController extends PhabricatorController { array_keys($read)); $this->setReadConpherences($read_conpherences); - if (!$this->getSelectedConpherencePHID()) { - $this->setSelectedConpherencePHID(reset($all_conpherence_phids)); - } - return $this; } @@ -141,13 +128,7 @@ abstract class ConpherenceController extends PhabricatorController { $user = $this->getRequest()->getUser(); foreach ($conpherences as $conpherence) { - $selected = false; - if ($this->getSelectedConpherencePHID() == $conpherence->getPHID()) { - $selected = true; - } - $item = $this->buildConpherenceMenuItem( - $conpherence, - $selected); + $item = $this->buildConpherenceMenuItem($conpherence); $nav->addCustomBlock($item->render()); } @@ -189,9 +170,7 @@ abstract class ConpherenceController extends PhabricatorController { return $crumbs; } - protected function buildConpherenceMenuItem( - $conpherence, - $selected) { + protected function buildConpherenceMenuItem($conpherence) { $user = $this->getRequest()->getUser(); $uri = $this->getApplicationURI('view/'.$conpherence->getID().'/'); @@ -216,13 +195,11 @@ abstract class ConpherenceController extends PhabricatorController { ->setUnreadCount($unread_count) ->setID($conpherence->getPHID().'-nav-item') ->addSigil('conpherence-menu-click') - ->setMetadata(array('id' => $conpherence->getID())); - - if ($selected) { - $item - ->addClass('conpherence-selected') - ->addClass('hide-unread-count'); - } + ->setMetadata( + array( + 'id' => $conpherence->getID(), + 'phid' => $conpherence->getPHID(), + )); return $item; } diff --git a/src/applications/conpherence/controller/ConpherenceListController.php b/src/applications/conpherence/controller/ConpherenceListController.php index ff1c6e29aa..7b9af17098 100644 --- a/src/applications/conpherence/controller/ConpherenceListController.php +++ b/src/applications/conpherence/controller/ConpherenceListController.php @@ -27,6 +27,7 @@ final class ConpherenceListController extends $conpherence_id = $this->getConpherenceID(); $current_selection_epoch = null; + $selected_phid = null; if ($conpherence_id) { $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) @@ -39,7 +40,7 @@ final class ConpherenceListController extends if ($conpherence->getTitle()) { $title = $conpherence->getTitle(); } - $this->setSelectedConpherencePHID($conpherence->getPHID()); + $selected_phid = $conpherence->getPHID(); $participant = $conpherence->getParticipant($user->getPHID()); $current_selection_epoch = $participant->getDateTouched(); @@ -50,7 +51,7 @@ final class ConpherenceListController extends $main_pane = id(new ConpherenceLayoutView()) ->setBaseURI($this->getApplicationURI()) - ->setSelectedConpherencePHID($this->getSelectedConpherencePHID()); + ->setSelectedConpherencePHID($selected_phid); $nav->appendChild( array( diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 7247aa394f..2e00c65f8b 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -4,20 +4,73 @@ * javelin-dom * javelin-request * javelin-stratcom - * javelin-uri - * javelin-util * javelin-workflow + * javelin-behavior-device */ JX.behavior('conpherence-menu', function(config) { - function onwidgetresponse(context, response) { + var thread = { + selected: null, + node: null, + visible: null + }; + + function selectthread(node) { + if (node === thread.node) { + return; + } + + if (thread.node) { + JX.DOM.alterClass(thread.node, 'conpherence-selected', false); + JX.DOM.alterClass(thread.node, 'hide-unread-count', false); + } + + JX.DOM.alterClass(node, 'conpherence-selected', true); + JX.DOM.alterClass(node, 'hide-unread-count', true); + + thread.node = node; + + var data = JX.Stratcom.getData(node); + thread.selected = data.phid; + + // TODO: These URIs don't work yet, so don't push them until they do. + // JX.History.push(config.base_uri + 'view/' + data.id + '/'); + + redrawthread(); + } + + function redrawthread() { + if (!thread.node) { + return; + } + if (thread.visible == thread.selected) { + return; + } + + var data = JX.Stratcom.getData(thread.node); + + var uri = config.base_uri + 'view/' + data.id + '/'; + var widget_uri = config.base_uri + 'widget/' + data.id + '/'; + + new JX.Workflow(uri, {}) + .setHandler(onresponse) + .start(); + + new JX.Workflow(widget_uri, {}) + .setHandler(onwidgetresponse) + .start(); + + thread.visible = thread.selected; + } + + function onwidgetresponse(response) { var widgets = JX.$H(response.widgets); var widgetsRoot = JX.$(config.widgets_pane); JX.DOM.setContent(widgetsRoot, widgets); } - function onresponse(context, response) { + function onresponse(response) { var header = JX.$H(response.header); var messages = JX.$H(response.messages); var form = JX.$H(response.form); @@ -30,30 +83,6 @@ JX.behavior('conpherence-menu', function(config) { JX.DOM.setContent(messagesRoot, messages); messagesRoot.scrollTop = messagesRoot.scrollHeight; JX.DOM.setContent(formRoot, form); - - var conpherences = JX.DOM.scry( - menuRoot, - 'a', - 'conpherence-menu-click' - ); - - for (var i = 0; i < conpherences.length; i++) { - var current = conpherences[i]; - if (current.id == context.id) { - JX.DOM.alterClass(current, 'conpherence-selected', true); - JX.DOM.alterClass(current, 'hide-unread-count', true); - } else { - JX.DOM.alterClass(current, 'conpherence-selected', false); - } - } - - // TODO - update the browser URI T2086 - - JX.Stratcom.invoke( - 'conpherence-selected-loaded', - null, - {} - ); } JX.Stratcom.listen( @@ -61,52 +90,8 @@ JX.behavior('conpherence-menu', function(config) { 'conpherence-menu-click', function(e) { e.kill(); - var selected = e.getNode(['conpherence-menu-click']); - if (config.fancy_ajax) { - JX.Stratcom.invoke( - 'conpherence-selected', - null, - { selected : selected } - ); - } else { - var data = JX.Stratcom.getData(selected); - var uri = new JX.URI(config.base_uri + data.id + '/'); - uri.go(); - } - } - ); - - JX.Stratcom.listen( - 'conpherence-initial-selected', - null, - function(e) { - var selected = e.getData().selected; - e.kill(); - JX.Stratcom.invoke( - 'conpherence-selected', - null, - { selected : selected } - ); - } - ); - - JX.Stratcom.listen( - 'conpherence-selected', - null, - function(e) { - - var selected = e.getData().selected; - var data = JX.Stratcom.getData(selected); - var uri = config.base_uri + 'view/' + data.id + '/'; - var widget_uri = config.base_uri + 'widget/' + data.id + '/'; - new JX.Workflow(uri, {}) - .setHandler(JX.bind(null, onresponse, selected)) - .start(); - new JX.Workflow(widget_uri, {}) - .setHandler(JX.bind(null, onwidgetresponse, selected)) - .start(); - } - ); + selectthread(e.getNode('conpherence-menu-click')); + }); JX.Stratcom.listen('click', 'conpherence-edit-metadata', function (e) { e.kill(); @@ -143,16 +128,42 @@ JX.behavior('conpherence-menu', function(config) { }).setData({ offset: last_offset+1 }).send(); }); - // select the current message - var selectedConpherence = false; + + // On mobile, we just show a thread list, so we don't want to automatically + // select or load any threads. On Desktop, we automatically select the first + // thread. + + function ondevicechange() { + if (JX.Device.getDevice() != 'desktop') { + return; + } + + // If there's no thread selected yet, select the first thread. + if (!thread.selected) { + var threads = JX.DOM.scry(document.body, 'a', 'conpherence-menu-click'); + if (threads.length) { + selectthread(threads[0]); + } + } + + // We might have a selected but undrawn thread for + redrawthread(); + } + + JX.Stratcom.listen('phabricator-device-change', null, ondevicechange); + ondevicechange(); + + + // If there's a currently visible thread, select it. if (config.selected_conpherence_id) { - var selected = JX.$(config.selected_conpherence_id + '-nav-item'); - JX.Stratcom.invoke( - 'conpherence-initial-selected', - null, - { selected : selected } - ); - selectedConpherence = true; + var threads = JX.DOM.scry(document.body, 'a', 'conpherence-menu-click'); + for (var ii = 0; ii < threads.length; ii++) { + var data = JX.Stratcom.getData(threads[ii]); + if (data.phid == config.selected_conpherence_id) { + selectthread(threads[ii]); + break; + } + } } });