From 404fe482d91492308dc839b5635de579b17dff5b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 8 May 2017 07:58:02 -0700 Subject: [PATCH] Add a Quicksand-aware page-level container to diffs Summary: Ref T12616. Ref T8047. Ref T11093. We currently have several bugs where diff state sticks across Quicksand pages. Add a new top-level object to handle this, with `sleep()` and `wake()` methods. In `sleep()`, future changes will remove/deacivate all the reticles/editors/etc. See T12616 for high-level discussion of plans here. This general idea is likely to become more formal eventually (e.g. for "sheets" or whatever we call them, in T10469) but I think this is probably a reasonable place to draw a line for now. Test Plan: - Added some logging to sleep(), wake() and construct(). - Viewed changes in Differential. - With Quicksand on, browsed around; saw state change logs fire properly. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12616, T11093, T8047 Differential Revision: https://secure.phabricator.com/D17840 --- resources/celerity/map.php | 26 ++++---- .../js/application/diff/DiffChangesetList.js | 25 ++++++++ .../differential/behavior-populate.js | 60 ++++++++++++++++++- 3 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 webroot/rsrc/js/application/diff/DiffChangesetList.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 8430bf43b9..a2933b83aa 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => '2ff7879f', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '90b30783', - 'differential.pkg.js' => 'ddfeb49b', + 'differential.pkg.js' => '84d27954', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -390,6 +390,7 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', + 'rsrc/js/application/diff/DiffChangesetList.js' => '58c4c0d6', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/ChangesetViewManager.js' => 'a2828756', 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '2e3f9738', @@ -399,7 +400,7 @@ return array( 'rsrc/js/application/differential/behavior-dropdown-menus.js' => '9a6b9324', 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '4fbbc3e9', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '92904457', - 'rsrc/js/application/differential/behavior-populate.js' => '8694b1df', + 'rsrc/js/application/differential/behavior-populate.js' => 'c0c44c3e', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => 'c93358e3', @@ -628,7 +629,7 @@ return array( 'javelin-behavior-differential-edit-inline-comments' => '4fbbc3e9', 'javelin-behavior-differential-feedback-preview' => 'b064af76', 'javelin-behavior-differential-keyboard-navigation' => '92904457', - 'javelin-behavior-differential-populate' => '8694b1df', + 'javelin-behavior-differential-populate' => 'c0c44c3e', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', 'javelin-behavior-diffusion-browse-file' => '054a0f0b', @@ -786,6 +787,7 @@ return array( 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', + 'phabricator-diff-changeset-list' => '58c4c0d6', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -1353,6 +1355,9 @@ return array( 'javelin-vector', 'javelin-dom', ), + '58c4c0d6' => array( + 'javelin-install', + ), '58dea2fa' => array( 'javelin-install', 'javelin-util', @@ -1558,13 +1563,6 @@ return array( 'phabricator-notification', 'conpherence-thread-manager', ), - '8694b1df' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'phabricator-tooltip', - 'changeset-view-manager', - ), '88236f00' => array( 'javelin-behavior', 'phabricator-keyboard-shortcut', @@ -1948,6 +1946,14 @@ return array( 'javelin-install', 'javelin-dom', ), + 'c0c44c3e' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'phabricator-tooltip', + 'changeset-view-manager', + 'phabricator-diff-changeset-list', + ), 'c420b0b9' => array( 'javelin-behavior', 'javelin-behavior-device', diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js new file mode 100644 index 0000000000..d62c2abd7a --- /dev/null +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -0,0 +1,25 @@ +/** + * @provides phabricator-diff-changeset-list + * @requires javelin-install + * @javelin + */ + +JX.install('DiffChangesetList', { + + construct: function() { + + }, + + members: { + + sleep: function() { + + }, + + wake: function() { + + } + + } + +}); diff --git a/webroot/rsrc/js/application/differential/behavior-populate.js b/webroot/rsrc/js/application/differential/behavior-populate.js index 6d0aabb213..a7e54ddd37 100644 --- a/webroot/rsrc/js/application/differential/behavior-populate.js +++ b/webroot/rsrc/js/application/differential/behavior-populate.js @@ -5,9 +5,67 @@ * javelin-stratcom * phabricator-tooltip * changeset-view-manager + * phabricator-diff-changeset-list + * @javelin */ -JX.behavior('differential-populate', function(config) { +JX.behavior('differential-populate', function(config, statics) { + + // When we perform a Quicksand navigation, deactivate the changeset lists on + // the current page and activate the changeset lists on the new page. + var onredraw = function(page_id) { + // If the current page is already active, we don't need to do anything. + if (statics.pageID === page_id) { + return; + } + + var ii; + + // Put the old lists to sleep. + var old_lists = get_lists(statics.pageID); + for (ii = 0; ii < old_lists.length; ii++) { + old_lists[ii].sleep(); + } + statics.pageID = null; + + // Awaken the new lists, if they exist. + if (statics.pages.hasOwnProperty(page_id)) { + var new_lists = get_lists(page_id); + for (ii = 0; ii < new_lists.length; ii++) { + new_lists[ii].wake(); + } + + statics.pageID = page_id; + } + }; + + // Get changeset lists on the current page. + var get_lists = function(page_id) { + if (page_id === null) { + return []; + } + + return statics.pages[page_id] || []; + }; + + if (!statics.installed) { + statics.installed = true; + statics.pages = {}; + statics.pageID = null; + + JX.Stratcom.listen('quicksand-redraw', null, function(e) { + onredraw(e.getData().newResponseID); + }); + } + + var changeset_list = new JX.DiffChangesetList(); + + // Install and activate the current page. + var page_id = JX.Quicksand.getCurrentPageID(); + statics.pages[page_id] = [changeset_list]; + onredraw(page_id); + + for (var ii = 0; ii < config.changesetViewIDs.length; ii++) { var id = config.changesetViewIDs[ii];