From 1855ed4bee4020b35283a8bce5f68302e56edee3 Mon Sep 17 00:00:00 2001 From: Alan Huang Date: Tue, 14 Aug 2012 14:03:26 -0700 Subject: [PATCH] Support symbol linking in Remarkup code blocks Summary: Trigger the crossreference behavior on code blocks. Limited to Differential, where we know what the project is, but includes regular comments, inline comments, and previews of both. (Hopefully event handlers on deleted elements also get deleted, so we don't leak memory? Also, caching is a problem, and I didn't find a way to mark existing cache entries as stale, like `DifferentialChangesetParser::CACHE_VERSION`...) Test Plan: Load Differential revision, make lots of comments, click on things. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T1602 Differential Revision: https://secure.phabricator.com/D3283 --- src/__celerity_resource_map__.php | 44 ++++----- .../DifferentialRevisionViewController.php | 16 +++- .../DifferentialRevisionCommentListView.php | 23 +++-- .../differential/behavior-comment-preview.js | 9 +- .../repository/repository-crossreference.js | 94 ++++++++++++------- 5 files changed, 119 insertions(+), 67 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 6630b9b9d9..a5d9796e7f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1306,7 +1306,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-feedback-preview' => array( - 'uri' => '/res/f27f3f49/rsrc/js/application/differential/behavior-comment-preview.js', + 'uri' => '/res/d6c8a84c/rsrc/js/application/differential/behavior-comment-preview.js', 'type' => 'js', 'requires' => array( @@ -1860,7 +1860,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-repository-crossreference' => array( - 'uri' => '/res/b24ebee5/rsrc/js/application/repository/repository-crossreference.js', + 'uri' => '/res/d3ff7611/rsrc/js/application/repository/repository-crossreference.js', 'type' => 'js', 'requires' => array( @@ -3119,7 +3119,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/19ebcc79/differential.pkg.css', 'type' => 'css', ), - 29296904 => + '5b33c790' => array( 'name' => 'differential.pkg.js', 'symbols' => @@ -3143,7 +3143,7 @@ celerity_register_resource_map(array( 16 => 'javelin-behavior-differential-dropdown-menus', 17 => 'javelin-behavior-buoyant', ), - 'uri' => '/res/pkg/29296904/differential.pkg.js', + 'uri' => '/res/pkg/5b33c790/differential.pkg.js', 'type' => 'js', ), 'c8ce2d88' => @@ -3251,7 +3251,7 @@ celerity_register_resource_map(array( 'aphront-typeahead-control-css' => 'edf6b149', 'differential-changeset-view-css' => '19ebcc79', 'differential-core-view-css' => '19ebcc79', - 'differential-inline-comment-editor' => '29296904', + 'differential-inline-comment-editor' => '5b33c790', 'differential-local-commits-view-css' => '19ebcc79', 'differential-results-table-css' => '19ebcc79', 'differential-revision-add-comment-css' => '19ebcc79', @@ -3264,21 +3264,21 @@ celerity_register_resource_map(array( 'inline-comment-summary-css' => '19ebcc79', 'javelin-behavior' => '6fb20113', 'javelin-behavior-aphront-basic-tokenizer' => '97f65640', - 'javelin-behavior-aphront-drag-and-drop' => '29296904', - 'javelin-behavior-aphront-drag-and-drop-textarea' => '29296904', + 'javelin-behavior-aphront-drag-and-drop' => '5b33c790', + 'javelin-behavior-aphront-drag-and-drop-textarea' => '5b33c790', 'javelin-behavior-aphront-form-disable-on-submit' => '971b021e', 'javelin-behavior-audit-preview' => '5e68be89', - 'javelin-behavior-buoyant' => '29296904', - 'javelin-behavior-differential-accept-with-errors' => '29296904', - 'javelin-behavior-differential-add-reviewers-and-ccs' => '29296904', - 'javelin-behavior-differential-comment-jump' => '29296904', - 'javelin-behavior-differential-diff-radios' => '29296904', - 'javelin-behavior-differential-dropdown-menus' => '29296904', - 'javelin-behavior-differential-edit-inline-comments' => '29296904', - 'javelin-behavior-differential-feedback-preview' => '29296904', - 'javelin-behavior-differential-keyboard-navigation' => '29296904', - 'javelin-behavior-differential-populate' => '29296904', - 'javelin-behavior-differential-show-more' => '29296904', + 'javelin-behavior-buoyant' => '5b33c790', + 'javelin-behavior-differential-accept-with-errors' => '5b33c790', + 'javelin-behavior-differential-add-reviewers-and-ccs' => '5b33c790', + 'javelin-behavior-differential-comment-jump' => '5b33c790', + 'javelin-behavior-differential-diff-radios' => '5b33c790', + 'javelin-behavior-differential-dropdown-menus' => '5b33c790', + 'javelin-behavior-differential-edit-inline-comments' => '5b33c790', + 'javelin-behavior-differential-feedback-preview' => '5b33c790', + 'javelin-behavior-differential-keyboard-navigation' => '5b33c790', + 'javelin-behavior-differential-populate' => '5b33c790', + 'javelin-behavior-differential-show-more' => '5b33c790', 'javelin-behavior-diffusion-commit-graph' => '5e68be89', 'javelin-behavior-diffusion-pull-lastmodified' => '5e68be89', 'javelin-behavior-maniphest-batch-selector' => '7707de41', @@ -3288,12 +3288,12 @@ celerity_register_resource_map(array( 'javelin-behavior-maniphest-transaction-preview' => '7707de41', 'javelin-behavior-phabricator-autofocus' => '971b021e', 'javelin-behavior-phabricator-keyboard-shortcuts' => '971b021e', - 'javelin-behavior-phabricator-object-selector' => '29296904', + 'javelin-behavior-phabricator-object-selector' => '5b33c790', 'javelin-behavior-phabricator-oncopy' => '971b021e', 'javelin-behavior-phabricator-tooltips' => '971b021e', 'javelin-behavior-phabricator-watch-anchor' => '971b021e', 'javelin-behavior-refresh-csrf' => '971b021e', - 'javelin-behavior-repository-crossreference' => '29296904', + 'javelin-behavior-repository-crossreference' => '5b33c790', 'javelin-behavior-workflow' => '971b021e', 'javelin-dom' => '6fb20113', 'javelin-event' => '6fb20113', @@ -3319,7 +3319,7 @@ celerity_register_resource_map(array( 'phabricator-core-buttons-css' => 'edf6b149', 'phabricator-core-css' => 'edf6b149', 'phabricator-directory-css' => 'edf6b149', - 'phabricator-drag-and-drop-file-upload' => '29296904', + 'phabricator-drag-and-drop-file-upload' => '5b33c790', 'phabricator-dropdown-menu' => '971b021e', 'phabricator-flag-css' => 'edf6b149', 'phabricator-jump-nav' => 'edf6b149', @@ -3331,7 +3331,7 @@ celerity_register_resource_map(array( 'phabricator-prefab' => '971b021e', 'phabricator-project-tag-css' => '7839ae2d', 'phabricator-remarkup-css' => 'edf6b149', - 'phabricator-shaped-request' => '29296904', + 'phabricator-shaped-request' => '5b33c790', 'phabricator-standard-page-view' => 'edf6b149', 'phabricator-tooltip' => '971b021e', 'phabricator-transaction-view-css' => 'edf6b149', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 28db134b48..b7a118efc4 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -256,11 +256,12 @@ final class DifferentialRevisionViewController extends DifferentialController { DifferentialChangesetParser::WHITESPACE_IGNORE_ALL); if ($arc_project) { - $symbol_indexes = $this->buildSymbolIndexes( + list($symbol_indexes, $project_phids) = $this->buildSymbolIndexes( $arc_project, $visible_changesets); } else { $symbol_indexes = array(); + $project_phids = null; } $revision_detail->setActions($actions); @@ -275,6 +276,15 @@ final class DifferentialRevisionViewController extends DifferentialController { $comment_view->setTargetDiff($target); $comment_view->setVersusDiffID($diff_vs); + if ($arc_project) { + Javelin::initBehavior( + 'repository-crossreference', + array( + 'section' => $comment_view->getID(), + 'projects' => $project_phids, + )); + } + $changeset_view = new DifferentialChangesetListView(); $changeset_view->setChangesets($changesets); $changeset_view->setVisibleChangesets($visible_changesets); @@ -777,7 +787,7 @@ final class DifferentialRevisionViewController extends DifferentialController { $langs = $arc_project->getSymbolIndexLanguages(); if (!$langs) { - return array(); + return array(array(), array()); } $symbol_indexes = array(); @@ -797,7 +807,7 @@ final class DifferentialRevisionViewController extends DifferentialController { } } - return $symbol_indexes; + return array($symbol_indexes, $project_phids); } private function loadOtherRevisions( diff --git a/src/applications/differential/view/DifferentialRevisionCommentListView.php b/src/applications/differential/view/DifferentialRevisionCommentListView.php index c65b9ffd9b..d1bbf4c1a1 100644 --- a/src/applications/differential/view/DifferentialRevisionCommentListView.php +++ b/src/applications/differential/view/DifferentialRevisionCommentListView.php @@ -25,6 +25,7 @@ final class DifferentialRevisionCommentListView extends AphrontView { private $user; private $target; private $versusDiffID; + private $id; public function setComments(array $comments) { assert_instances_of($comments, 'DifferentialComment'); @@ -65,6 +66,13 @@ final class DifferentialRevisionCommentListView extends AphrontView { return $this; } + public function getID() { + if (!$this->id) { + $this->id = celerity_generate_unique_node_id(); + } + return $this->id; + } + public function render() { require_celerity_resource('differential-revision-comment-list-css'); @@ -179,11 +187,14 @@ final class DifferentialRevisionCommentListView extends AphrontView { $hidden = null; } - return - '
'. - implode("\n", $header). - $hidden. - implode("\n", $visible). - '
'; + return javelin_render_tag( + 'div', + array( + 'class' => 'differential-comment-list', + 'id' => $this->getID(), + ), + implode("\n", $header). + $hidden. + implode("\n", $visible)); } } diff --git a/webroot/rsrc/js/application/differential/behavior-comment-preview.js b/webroot/rsrc/js/application/differential/behavior-comment-preview.js index 104c6e072d..3fd78397fb 100644 --- a/webroot/rsrc/js/application/differential/behavior-comment-preview.js +++ b/webroot/rsrc/js/application/differential/behavior-comment-preview.js @@ -19,7 +19,11 @@ JX.behavior('differential-feedback-preview', function(config) { } var callback = function(r) { - JX.DOM.setContent(JX.$(config.preview), JX.$H(r)); + var preview = JX.$(config.preview); + JX.DOM.setContent(preview, JX.$H(r)); + JX.Stratcom.invoke('differential-preview-update', null, { + container: preview + }); }; var getdata = function() { @@ -50,6 +54,9 @@ JX.behavior('differential-feedback-preview', function(config) { var inline = JX.$(config.inline); JX.DOM.setContent(inline, JX.$H(r)); + JX.Stratcom.invoke('differential-preview-update', null, { + container: inline + }); // Go through the previews and activate any "View" links where the // actual comment appears in the document. diff --git a/webroot/rsrc/js/application/repository/repository-crossreference.js b/webroot/rsrc/js/application/repository/repository-crossreference.js index 076a535d1b..d4e88315bd 100644 --- a/webroot/rsrc/js/application/repository/repository-crossreference.js +++ b/webroot/rsrc/js/application/repository/repository-crossreference.js @@ -10,43 +10,67 @@ JX.behavior('repository-crossreference', function(config) { // NOTE: Pretty much everything in this file is a worst practice. We're // constrained by the markup generated by the syntax highlighters. - var container = JX.$(config.container); - JX.DOM.alterClass(container, 'repository-crossreference', true); - JX.DOM.listen( - container, - 'click', - 'tag:span', - function(e) { - if (window.getSelection && !window.getSelection().isCollapsed) { - return; - } - var target = e.getTarget(); - var map = {nc : 'class', nf : 'function', na : null}; - while (target !== document.body) { - if (JX.DOM.isNode(target, 'span') && (target.className in map)) { - var symbol = target.textContent || target.innerText; - var query = { - lang : config.lang, - projects : config.projects.join(','), - jump : true - }; - if (map[target.className]) { - query.type = map[target.className]; - } - if (target.hasAttribute('data-symbol-context')) { - query.context = target.getAttribute('data-symbol-context'); - } - if (target.hasAttribute('data-symbol-name')) { - symbol = target.getAttribute('data-symbol-name'); - } - var uri = JX.$U('/diffusion/symbol/' + symbol + '/'); - uri.addQueryParams(query); - window.open(uri); - e.kill(); - break; + function link(element, lang) { + JX.DOM.alterClass(element, 'repository-crossreference', true); + JX.DOM.listen( + element, + 'click', + 'tag:span', + function(e) { + if (window.getSelection && !window.getSelection().isCollapsed) { + return; } - target = target.parentNode; + var target = e.getTarget(); + var map = {nc : 'class', nf : 'function', na : null}; + while (target !== document.body) { + if (JX.DOM.isNode(target, 'span') && (target.className in map)) { + var symbol = target.textContent || target.innerText; + var query = { + lang : lang, + projects : config.projects.join(','), + jump : true + }; + if (map[target.className]) { + query.type = map[target.className]; + } + if (target.hasAttribute('data-symbol-context')) { + query.context = target.getAttribute('data-symbol-context'); + } + if (target.hasAttribute('data-symbol-name')) { + symbol = target.getAttribute('data-symbol-name'); + } + var uri = JX.$U('/diffusion/symbol/' + symbol + '/'); + uri.addQueryParams(query); + window.open(uri); + e.kill(); + break; + } + target = target.parentNode; + } + }); + } + + function linkAll(section) { + var blocks = section.getElementsByClassName('remarkup-code-block'); + for (var i = 0; i < blocks.length; ++i) { + if (blocks[i].hasAttribute('data-code-lang')) { + var lang = blocks[i].getAttribute('data-code-lang'); + link(blocks[i], lang); } + } + } + + if (config.container) { + link(JX.$(config.container), config.lang); + } else if (config.section) { + linkAll(JX.$(config.section)); + } + + JX.Stratcom.listen( + 'differential-preview-update', + null, + function(e) { + linkAll(e.getData().container); }); });