From b804e8cffad25019c5b4ebc26cc85bd3b41b9537 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 8 May 2020 07:27:43 -0700 Subject: [PATCH] Make "View" from inline comment previews correctly jump to "isEditing" inlines Summary: Ref T13513. Currently, clicking "View" from the inline comment preview (below the "add comment" area at the bottom of the page) only works if the inline isn't being edited. Update this behavior so it works on inlines in either "Viewing" or "Editing" states. Test Plan: - Clicked "View" on a normal inline, got jumped/selected. - Clicked "View" on an editing inline, got jumped/selected. Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21237 --- resources/celerity/map.php | 24 ++++------ resources/celerity/packages.php | 1 - .../view/PHUIDiffInlineCommentDetailView.php | 6 +-- .../PHUIDiffInlineCommentPreviewListView.php | 8 ---- .../js/application/diff/DiffChangesetList.js | 46 +++++++++++++++---- .../application/diff/behavior-preview-link.js | 36 --------------- 6 files changed, 48 insertions(+), 73 deletions(-) delete mode 100644 webroot/rsrc/js/application/diff/behavior-preview-link.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 5a84734132..808673971c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => '1e667bcb', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => 'd71d4531', - 'differential.pkg.js' => 'cf4f3263', + 'differential.pkg.js' => '30307170', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -380,11 +380,10 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => '700bf848', - 'rsrc/js/application/diff/DiffChangesetList.js' => '6992b85c', + 'rsrc/js/application/diff/DiffChangesetList.js' => '6e668c5b', 'rsrc/js/application/diff/DiffInline.js' => 'db754a7b', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', - 'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', 'rsrc/js/application/differential/behavior-populate.js' => 'b86ef6c2', 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => '94243d89', @@ -612,7 +611,6 @@ return array( 'javelin-behavior-desktop-notifications-control' => '070679fe', 'javelin-behavior-detect-timezone' => '78bc5d94', 'javelin-behavior-device' => '0cf79f45', - 'javelin-behavior-diff-preview-link' => 'f51e9c17', 'javelin-behavior-differential-diff-radios' => '925fe8cd', 'javelin-behavior-differential-populate' => 'b86ef6c2', 'javelin-behavior-diffusion-commit-branches' => '4b671572', @@ -777,7 +775,7 @@ return array( 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => '700bf848', - 'phabricator-diff-changeset-list' => '6992b85c', + 'phabricator-diff-changeset-list' => '6e668c5b', 'phabricator-diff-inline' => 'db754a7b', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', @@ -1511,11 +1509,6 @@ return array( 'javelin-install', 'javelin-dom', ), - '6992b85c' => array( - 'javelin-install', - 'phuix-button-view', - 'phabricator-diff-tree-view', - ), '6a1583a8' => array( 'javelin-behavior', 'javelin-history', @@ -1552,6 +1545,11 @@ return array( 'javelin-install', 'javelin-util', ), + '6e668c5b' => array( + 'javelin-install', + 'phuix-button-view', + 'phabricator-diff-tree-view', + ), '700bf848' => array( 'javelin-dom', 'javelin-util', @@ -2174,11 +2172,6 @@ return array( 'javelin-dom', 'javelin-vector', ), - 'f51e9c17' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - ), 'f84bcbf4' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2429,7 +2422,6 @@ return array( 'phuix-formation-view', 'phuix-formation-column-view', 'phuix-formation-flank-view', - 'javelin-behavior-diff-preview-link', ), 'diffusion.pkg.css' => array( 'diffusion-icons-css', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index 8817518186..a3df0dd154 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -220,7 +220,6 @@ return array( 'phuix-formation-view', 'phuix-formation-column-view', 'phuix-formation-flank-view', - 'javelin-behavior-diff-preview-link', ), 'diffusion.pkg.css' => array( 'diffusion-icons-css', diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php index 2d3a9870df..77b28bb336 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -212,10 +212,10 @@ final class PHUIDiffInlineCommentDetailView 'a', array( 'class' => 'inline-button-divider pml msl', - 'meta' => array( - 'anchor' => $anchor_name, + 'meta' => array( + 'inlineCommentID' => $inline->getID(), ), - 'sigil' => 'differential-inline-preview-jump', + 'sigil' => 'differential-inline-preview-jump', ), pht('View')); diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php index 3f42244600..9507004520 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php @@ -28,14 +28,6 @@ final class PHUIDiffInlineCommentPreviewListView public function render() { $viewer = $this->getViewer(); - $config = array( - 'pht' => array( - 'view' => pht('View'), - ), - ); - - Javelin::initBehavior('diff-preview-link', $config); - $inlines = $this->getInlineComments(); foreach ($inlines as $key => $inline) { $inlines[$key] = $inline->newInlineCommentObject(); diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index c806d81647..f7ebb1bf85 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -1115,17 +1115,19 @@ JX.install('DiffChangesetList', { this.selectInline(inline); }, - selectInline: function(inline) { + selectInline: function(inline, force, scroll) { var selection = this._getSelectionState(); var item; - // If the comment the user clicked is currently selected, deselect it. - // This makes it easy to undo things if you clicked by mistake. - if (selection.cursor !== null) { - item = selection.items[selection.cursor]; - if (item.target === inline) { - this._setSelectionState(null, false); - return; + if (!force) { + // If the comment the user clicked is currently selected, deselect it. + // This makes it easy to undo things if you clicked by mistake. + if (selection.cursor !== null) { + item = selection.items[selection.cursor]; + if (item.target === inline) { + this._setSelectionState(null, false); + return; + } } } @@ -1136,9 +1138,10 @@ JX.install('DiffChangesetList', { for (var ii = 0; ii < items.length; ii++) { item = items[ii]; if (item.target === inline) { - this._setSelectionState(item, false); + this._setSelectionState(item, scroll); } } + }, redrawPreview: function() { @@ -2117,6 +2120,31 @@ JX.install('DiffChangesetList', { ['differential-inline-comment', 'tag:textarea'], ondraft); + var on_preview_view = JX.bind(this, this._onPreviewEvent, 'view'); + JX.Stratcom.listen( + 'click', + 'differential-inline-preview-jump', + on_preview_view); + }, + + _onPreviewEvent: function(action, e) { + if (this.isAsleep()) { + return; + } + + var data = e.getNodeData('differential-inline-preview-jump'); + var inline = this.getInlineByID(data.inlineCommentID); + if (!inline) { + return; + } + + e.kill(); + + switch (action) { + case 'view': + this.selectInline(inline, true, true); + break; + } }, _onInlineEvent: function(action, e) { diff --git a/webroot/rsrc/js/application/diff/behavior-preview-link.js b/webroot/rsrc/js/application/diff/behavior-preview-link.js deleted file mode 100644 index 71cd2ecf04..0000000000 --- a/webroot/rsrc/js/application/diff/behavior-preview-link.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * @provides javelin-behavior-diff-preview-link - * @requires javelin-behavior - * javelin-stratcom - * javelin-dom - */ - -JX.behavior('diff-preview-link', function(config, statics) { - if (statics.initialized) { - return; - } - statics.initialized = true; - - var pht = JX.phtize(config.pht); - - // After inline comment previews are rendered, hook up the links to the - // comments that are visible on the current page. - function link_inline_preview(e) { - var root = e.getData().rootNode; - var links = JX.DOM.scry(root, 'a', 'differential-inline-preview-jump'); - - for (var ii = 0; ii < links.length; ii++) { - var data = JX.Stratcom.getData(links[ii]); - try { - JX.$(data.anchor); - links[ii].href = '#' + data.anchor; - JX.DOM.setContent(links[ii], pht('view')); - } catch (ignored) { - // This inline comment isn't visible, e.g. on some other diff. - } - } - - } - - JX.Stratcom.listen('EditEngine.didCommentPreview', null, link_inline_preview); -}); -- 2.51.2