From 10f1692b1bac89db568bffd94e19de9a06ce0bcf Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 10 Mar 2015 13:53:30 -0700 Subject: [PATCH] Conpherence - more threadManager stuff and get scrolling working Summary: Ref T7014. The main conpherence view is kind of broken without this in subtle ways because of /conpherence/ versus /conpherence/x/ init'ing things differently; this fixes that. Moves more normal view conpherence logic into threadManager. Makes all the display code happen outside of threadManager, setting us up for some display manager later maybe. Test Plan: sent messages, updated title, etc and the messages pane auto scrolled correctly! Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7014 Differential Revision: https://secure.phabricator.com/D12035 --- resources/celerity/map.php | 52 +++++++-------- .../ConpherenceUpdateController.php | 5 ++ .../controller/ConpherenceViewController.php | 1 + .../view/ConpherenceDurableColumnView.php | 3 +- .../view/ConpherenceLayoutView.php | 8 +++ .../rsrc/externals/javelin/lib/Scrollbar.js | 9 +++ .../conpherence/ConpherenceThreadManager.js | 65 +++++++++---------- .../conpherence/behavior-durable-column.js | 54 +++++++++------ .../application/conpherence/behavior-menu.js | 59 +++++++++-------- 9 files changed, 147 insertions(+), 109 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 7fea049c43..42485f9aab 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -207,7 +207,7 @@ return array( 'rsrc/externals/javelin/lib/Resource.js' => '44959b73', 'rsrc/externals/javelin/lib/Routable.js' => 'b3e7d692', 'rsrc/externals/javelin/lib/Router.js' => '29274e2b', - 'rsrc/externals/javelin/lib/Scrollbar.js' => '1ed54a27', + 'rsrc/externals/javelin/lib/Scrollbar.js' => '1feea462', 'rsrc/externals/javelin/lib/URI.js' => '6eff08aa', 'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8', 'rsrc/externals/javelin/lib/WebSocket.js' => 'e292eaf4', @@ -351,8 +351,8 @@ return array( 'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', - 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'd0742f48', - 'rsrc/js/application/conpherence/behavior-durable-column.js' => '8cf41980', + 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '11e2dcf2', + 'rsrc/js/application/conpherence/behavior-durable-column.js' => '5fc7fac0', 'rsrc/js/application/conpherence/behavior-menu.js' => '6bc52765', 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '2c1cd7f5', @@ -517,7 +517,7 @@ return array( 'conpherence-menu-css' => 'c6ac5299', 'conpherence-message-pane-css' => '5930260a', 'conpherence-notification-css' => '04a6e10a', - 'conpherence-thread-manager' => 'd0742f48', + 'conpherence-thread-manager' => '11e2dcf2', 'conpherence-update-css' => '1099a660', 'conpherence-widget-pane-css' => '3d575438', 'differential-changeset-view-css' => '6a8b172a', @@ -584,7 +584,7 @@ return array( 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', 'javelin-behavior-doorkeeper-tag' => 'e5822781', - 'javelin-behavior-durable-column' => '8cf41980', + 'javelin-behavior-durable-column' => '5fc7fac0', 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-fancy-datepicker' => 'c51ae228', 'javelin-behavior-global-drag-and-drop' => '07f199d8', @@ -678,7 +678,7 @@ return array( 'javelin-resource' => '44959b73', 'javelin-routable' => 'b3e7d692', 'javelin-router' => '29274e2b', - 'javelin-scrollbar' => '1ed54a27', + 'javelin-scrollbar' => '1feea462', 'javelin-stratcom' => '6c53634d', 'javelin-tokenizer' => '7644823e', 'javelin-typeahead' => '70baed2f', @@ -899,6 +899,16 @@ return array( 'javelin-install', 'javelin-util', ), + '11e2dcf2' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), '13c739ea' => array( 'javelin-behavior', 'javelin-stratcom', @@ -952,7 +962,7 @@ return array( 'javelin-dom', 'javelin-reactor-dom', ), - '1ed54a27' => array( + '1feea462' => array( 'javelin-install', 'javelin-dom', 'javelin-stratcom', @@ -1209,6 +1219,15 @@ return array( 'javelin-dom', 'javelin-vector', ), + '5fc7fac0' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-scrollbar', + 'javelin-quicksand', + 'phabricator-keyboard-shortcut', + 'conpherence-thread-manager', + ), '5fefb143' => array( 'javelin-behavior', 'javelin-dom', @@ -1535,15 +1554,6 @@ return array( 'javelin-stratcom', 'javelin-behavior', ), - '8cf41980' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-scrollbar', - 'javelin-quicksand', - 'phabricator-keyboard-shortcut', - 'conpherence-thread-manager', - ), '8e1389b5' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1768,16 +1778,6 @@ return array( 'javelin-stratcom', 'phabricator-phtize', ), - 'd0742f48' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), 'd19198c8' => array( 'javelin-install', 'javelin-dom', diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 8e54a518f6..649b870dab 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -299,8 +299,12 @@ final class ConpherenceUpdateController $need_widget_data = false; $need_transactions = false; + $need_participant_cache = false; switch ($action) { case ConpherenceUpdateActions::METADATA: + $need_participant_cache = true; + $need_transactions = true; + break; case ConpherenceUpdateActions::LOAD: $need_transactions = true; break; @@ -319,6 +323,7 @@ final class ConpherenceUpdateController $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) ->setAfterTransactionID($latest_transaction_id) + ->needParticipantCache($need_participant_cache) ->needWidgetData($need_widget_data) ->needTransactions($need_transactions) ->withIDs(array($conpherence_id)) diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index ecba5c7f28..7466526323 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -67,6 +67,7 @@ final class ConpherenceViewController extends ->setHeader($header) ->setMessages($messages) ->setReplyForm($form) + ->setLatestTransactionID($data['latest_transaction_id']) ->setRole('thread'); return $this->buildApplicationPage( diff --git a/src/applications/conpherence/view/ConpherenceDurableColumnView.php b/src/applications/conpherence/view/ConpherenceDurableColumnView.php index 9478c5a564..17c7138b2e 100644 --- a/src/applications/conpherence/view/ConpherenceDurableColumnView.php +++ b/src/applications/conpherence/view/ConpherenceDurableColumnView.php @@ -73,10 +73,11 @@ final class ConpherenceDurableColumnView extends AphrontTagView { $transactions = $this->buildTransactions(); - $content = phutil_tag( + $content = javelin_tag( 'div', array( 'class' => 'conpherence-durable-column-main', + 'sigil' => 'conpherence-durable-column-main', ), phutil_tag( 'div', diff --git a/src/applications/conpherence/view/ConpherenceLayoutView.php b/src/applications/conpherence/view/ConpherenceLayoutView.php index f02fa5a118..92acf371fe 100644 --- a/src/applications/conpherence/view/ConpherenceLayoutView.php +++ b/src/applications/conpherence/view/ConpherenceLayoutView.php @@ -9,6 +9,7 @@ final class ConpherenceLayoutView extends AphrontView { private $header; private $messages; private $replyForm; + private $latestTransactionID; public function setMessages($messages) { $this->messages = $messages; @@ -49,6 +50,11 @@ final class ConpherenceLayoutView extends AphrontView { return $this; } + public function setLatestTransactionID($id) { + $this->latestTransactionID = $id; + return $this; + } + public function render() { require_celerity_resource('conpherence-menu-css'); require_celerity_resource('conpherence-message-pane-css'); @@ -71,6 +77,8 @@ final class ConpherenceLayoutView extends AphrontView { 'layoutID' => $layout_id, 'selectedID' => $selected_id, 'selectedThreadID' => $selected_thread_id, + 'selectedThreadPHID' => $this->thread->getPHID(), + 'latestTransactionID' => $this->latestTransactionID, 'role' => $this->role, 'hasThreadList' => (bool)$this->threadView, 'hasThread' => (bool)$this->messages, diff --git a/webroot/rsrc/externals/javelin/lib/Scrollbar.js b/webroot/rsrc/externals/javelin/lib/Scrollbar.js index 6f52811619..edf3991269 100644 --- a/webroot/rsrc/externals/javelin/lib/Scrollbar.js +++ b/webroot/rsrc/externals/javelin/lib/Scrollbar.js @@ -377,6 +377,15 @@ JX.install('Scrollbar', { clearTimeout(this._timeout); this._timeout = null; } + }, + + scrollTo: function(scroll) { + if (this._viewport !== null) { + this._viewport.scrollTop = scroll; + } else { + this._frame.scrollTop = scroll; + } + return this; } } diff --git a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js index 07bdb73fd1..35b4122f42 100644 --- a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js +++ b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js @@ -28,12 +28,13 @@ JX.install('ConpherenceThreadManager', { _latestTransactionID: null, _updating: null, _minimalDisplay: false, - _getMessagesNodeFunction: JX.bag, - _getTitleNodeFunction: JX.bag, _willLoadThreadCallback: JX.bag, _didLoadThreadCallback: JX.bag, + _didUpdateThreadCallback: JX.bag, _willSendMessageCallback: JX.bag, _didSendMessageCallback: JX.bag, + _willUpdateWorkflowCallback: JX.bag, + _didUpdateWorkflowCallback: JX.bag, setLoadThreadURI: function(uri) { this._loadThreadURI = uri; @@ -56,10 +57,20 @@ JX.install('ConpherenceThreadManager', { return this._loadedThreadID; }, + setLoadedThreadID: function(id) { + this._loadedThreadID = id; + return this; + }, + getLoadedThreadPHID: function() { return this._loadedThreadPHID; }, + setLoadedThreadPHID: function(phid) { + this._loadedThreadPHID = phid; + return this; + }, + getLatestTransactionID: function() { return this._latestTransactionID; }, @@ -69,24 +80,6 @@ JX.install('ConpherenceThreadManager', { return this; }, - setMessagesNodeFunction: function(callback) { - this._getMessagesNodeFunction = callback; - return this; - }, - - _getMessagesNode: function() { - return this._getMessagesNodeFunction(); - }, - - setTitleNodeFunction: function(callback) { - this._getTitleNodeFunction = callback; - return this; - }, - - _getTitleNode: function() { - return this._getTitleNodeFunction(); - }, - setMinimalDisplay: function(bool) { this._minimalDisplay = bool; return this; @@ -102,6 +95,11 @@ JX.install('ConpherenceThreadManager', { return this; }, + setDidUpdateThreadCallback: function(callback) { + this._didUpdateThreadCallback = callback; + return this; + }, + setWillSendMessageCallback: function(callback) { this._willSendMessageCallback = callback; return this; @@ -112,6 +110,16 @@ JX.install('ConpherenceThreadManager', { return this; }, + setWillUpdateWorkflowCallback: function(callback) { + this._willUpdateWorkflowCallback = callback; + return this; + }, + + setDidUpdateWorkflowCallback: function(callback) { + this._didUpdateWorkflowCallback = callback; + return this; + }, + _getParams: function(base_params) { if (this._minimalDisplay) { base_params.minimal_display = true; @@ -121,6 +129,7 @@ JX.install('ConpherenceThreadManager', { } return base_params; }, + start: function() { JX.Stratcom.listen( 'aphlict-server-message', @@ -142,7 +151,6 @@ JX.install('ConpherenceThreadManager', { // Message event for something we already know about. return; } - // If we're currently updating, wait for the update to complete. // If this notification tells us about a message which is newer than // the newest one we know to exist, keep track of it so we can @@ -170,10 +178,7 @@ JX.install('ConpherenceThreadManager', { .setData(params) .setHandler(JX.bind(this, function(r) { this._latestTransactionID = r.latest_transaction_id; - - var messages = this._getMessagesNode(); - JX.DOM.appendContent(messages, JX.$H(r.transactions)); - messages.scrollTop = messages.scrollHeight; + this._didUpdateThreadCallback(r); })); this.syncWorkflow(workflow, 'finally'); @@ -197,17 +202,12 @@ JX.install('ConpherenceThreadManager', { runUpdateWorkflowFromLink: function(link, params) { params = this._getParams(params); - + this._willUpdateWorkflowCallback(); var workflow = new JX.Workflow.newFromLink(link) .setData(params) .setHandler(JX.bind(this, function(r) { this._latestTransactionID = r.latest_transaction_id; - - var messages = this._getMessagesNode(); - JX.DOM.appendContent(messages, JX.$H(r.transactions)); - messages.scrollTop = messages.scrollHeight; - - JX.DOM.setContent(this._getTitleNode(), r.conpherence_title); + this._didUpdateWorkflowCallback(r); })); this.syncWorkflow(workflow, params.stage); }, @@ -230,7 +230,6 @@ JX.install('ConpherenceThreadManager', { var handler = JX.bind(this, function(r) { this._loadedThreadID = r.threadID; this._loadedThreadPHID = r.threadPHID; - this._loadThreadID = r.threadID; this._latestTransactionID = r.latestTransactionID; this._didLoadThreadCallback(r); diff --git a/webroot/rsrc/js/application/conpherence/behavior-durable-column.js b/webroot/rsrc/js/application/conpherence/behavior-durable-column.js index 8a74ef9167..2749c712ef 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-durable-column.js +++ b/webroot/rsrc/js/application/conpherence/behavior-durable-column.js @@ -13,12 +13,18 @@ JX.behavior('durable-column', function() { var show = false; var loadThreadID = null; + var scrollbar = null; var frame = JX.$('phabricator-standard-page'); var quick = JX.$('phabricator-standard-page-body'); - function _getColumnContentNode() { - return JX.$('conpherence-durable-column-content'); + function _getColumnNode() { + return JX.$('conpherence-durable-column'); + } + + function _getColumnScrollNode() { + var column = _getColumnNode(); + return JX.DOM.find(column, 'div', 'conpherence-durable-column-main'); } function _toggleColumn() { @@ -42,7 +48,7 @@ JX.behavior('durable-column', function() { .setHandler(_toggleColumn) .register(); - new JX.Scrollbar(_getColumnContentNode()); + scrollbar = new JX.Scrollbar(_getColumnScrollNode()); JX.Quicksand.start(); @@ -51,29 +57,33 @@ JX.behavior('durable-column', function() { */ var threadManager = new JX.ConpherenceThreadManager(); threadManager.setMinimalDisplay(true); - threadManager.setMessagesNodeFunction(_getColumnMessagesNode); - threadManager.setTitleNodeFunction(_getColumnTitleNode); threadManager.setLoadThreadURI('/conpherence/columnview/'); - threadManager.setWillLoadThreadCallback(function () { + threadManager.setWillLoadThreadCallback(function() { _markLoading(true); }); - threadManager.setDidLoadThreadCallback(function (r) { + threadManager.setDidLoadThreadCallback(function(r) { var column = _getColumnNode(); var new_column = JX.$H(r.content); JX.DOM.replace(column, new_column); JX.DOM.show(_getColumnNode()); - new JX.Scrollbar(_getColumnContentNode()); + var messages = _getColumnMessagesNode(); + scrollbar = new JX.Scrollbar(_getColumnScrollNode()); + scrollbar.scrollTo(messages.scrollHeight); _markLoading(false); loadThreadID = threadManager.getLoadedThreadID(); }); - threadManager.setWillSendMessageCallback(function () { - _markLoading(true); - }); - threadManager.setDidSendMessageCallback(function (r) { + threadManager.setDidUpdateThreadCallback(function(r) { var messages = _getColumnMessagesNode(); JX.DOM.appendContent(messages, JX.$H(r.transactions)); - var content = _getColumnContentNode(); - content.scrollTop = content.scrollHeight; + scrollbar.scrollTo(messages.scrollHeight); + }); + threadManager.setWillSendMessageCallback(function() { + _markLoading(true); + }); + threadManager.setDidSendMessageCallback(function(r) { + var messages = _getColumnMessagesNode(); + JX.DOM.appendContent(messages, JX.$H(r.transactions)); + scrollbar.scrollTo(messages.scrollHeight); var textarea = _getColumnTextareaNode(); textarea.value = ''; @@ -82,6 +92,15 @@ JX.behavior('durable-column', function() { _focusColumnTextareaNode(); }); + threadManager.setWillUpdateWorkflowCallback(function() { + JX.Stratcom.invoke('notification-panel-close'); + }); + threadManager.setDidUpdateWorkflowCallback(function(r) { + var messages = this._getMessagesNode(); + JX.DOM.appendContent(messages, JX.$H(r.transactions)); + scrollbar.scrollTo(messages.scrollHeight); + JX.DOM.setContent(_getColumnTitleNode(), r.conpherence_title); + }); threadManager.start(); JX.Stratcom.listen( @@ -96,7 +115,6 @@ JX.behavior('durable-column', function() { switch (action) { case 'metadata': - JX.Stratcom.invoke('notification-panel-close'); threadManager.runUpdateWorkflowFromLink( link, { @@ -106,7 +124,6 @@ JX.behavior('durable-column', function() { }); break; case 'add_person': - JX.Stratcom.invoke('notification-panel-close'); threadManager.runUpdateWorkflowFromLink( link, { @@ -124,10 +141,6 @@ JX.behavior('durable-column', function() { } }); - function _getColumnNode() { - return JX.$('conpherence-durable-column'); - } - function _getColumnBodyNode() { var column = JX.$('conpherence-durable-column'); return JX.DOM.find( @@ -136,7 +149,6 @@ JX.behavior('durable-column', function() { 'conpherence-durable-column-body'); } - function _getColumnMessagesNode() { var column = JX.$('conpherence-durable-column'); return JX.DOM.find( diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 4d085bec4a..e0b1abf34f 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -24,8 +24,31 @@ JX.behavior('conpherence-menu', function(config) { // TODO - move more logic into the ThreadManager var threadManager = new JX.ConpherenceThreadManager(); - threadManager.setMessagesNodeFunction(function () { - return JX.DOM.find(document, 'div', 'conpherence-messages'); + threadManager.setWillLoadThreadCallback(function() { + markThreadLoading(true); + }); + threadManager.setDidLoadThreadCallback(function(r) { + var header = JX.$H(r.header); + var messages = JX.$H(r.messages); + var form = JX.$H(r.form); + var root = JX.DOM.find(document, 'div', 'conpherence-layout'); + var header_root = JX.DOM.find(root, 'div', 'conpherence-header-pane'); + var messages_root = JX.DOM.find(root, 'div', 'conpherence-messages'); + var form_root = JX.DOM.find(root, 'div', 'conpherence-form'); + JX.DOM.setContent(header_root, header); + JX.DOM.setContent(messages_root, messages); + JX.DOM.setContent(form_root, form); + + markThreadLoading(false); + + didRedrawThread(true); + }); + threadManager.setDidUpdateThreadCallback(function(r) { + var root = JX.DOM.find(document, 'div', 'conpherence-layout'); + var messages_root = JX.DOM.find(root, 'div', 'conpherence-message-pane'); + var messages = JX.DOM.find(messages_root, 'div', 'conpherence-messages'); + JX.DOM.appendContent(messages, JX.$H(r.transactions)); + messages.scrollTop = messages.scrollHeight; }); threadManager.setWillSendMessageCallback(function () { var root = JX.DOM.find(document, 'div', 'conpherence-layout'); @@ -155,12 +178,13 @@ JX.behavior('conpherence-menu', function(config) { var data = JX.Stratcom.getData(_thread.node); if (_thread.visible !== null || !config.hasThread) { - markThreadLoading(true); - var uri = config.baseURI + data.threadID + '/'; - new JX.Workflow(uri, {}) - .setHandler(JX.bind(null, onLoadThreadResponse, data.threadID)) - .start(); + threadManager.setLoadThreadURI('/conpherence/' + data.threadID + '/'); + threadManager.loadThreadByID(data.threadID); } else if (config.hasThread) { + // we loaded the thread via the server so let the thread manager know + threadManager.setLoadedThreadID(config.selectedThreadID); + threadManager.setLoadedThreadPHID(config.selectedThreadPHID); + threadManager.setLatestTransactionID(config.latestTransactionID); _scrollMessageWindow(); _focusTextarea(); } else { @@ -270,27 +294,6 @@ JX.behavior('conpherence-menu', function(config) { return widget; } - function onLoadThreadResponse(thread_id, response) { - // we got impatient and this is no longer the right answer :/ - if (_thread.selected != thread_id) { - return; - } - var header = JX.$H(response.header); - var messages = JX.$H(response.messages); - var form = JX.$H(response.form); - var root = JX.DOM.find(document, 'div', 'conpherence-layout'); - var header_root = JX.DOM.find(root, 'div', 'conpherence-header-pane'); - var messages_root = JX.DOM.find(root, 'div', 'conpherence-messages'); - var form_root = JX.DOM.find(root, 'div', 'conpherence-form'); - JX.DOM.setContent(header_root, header); - JX.DOM.setContent(messages_root, messages); - JX.DOM.setContent(form_root, form); - - markThreadLoading(false); - - didRedrawThread(true); - } - /** * This function is a wee bit tricky. Internally, we want to scroll the * message window and let other stuff - notably widgets - redraw / build if