Page MenuHomePhabricator
Files F13187343

D12205.diff
No OneTemporary
Actions

D12205.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1165,7 +1165,9 @@
'PHUIFormLayoutView' => 'view/form/PHUIFormLayoutView.php',
'PHUIFormMultiSubmitControl' => 'view/form/control/PHUIFormMultiSubmitControl.php',
'PHUIFormPageView' => 'view/form/PHUIFormPageView.php',
+ 'PHUIHandleListView' => 'applications/phid/view/PHUIHandleListView.php',
'PHUIHandleTagListView' => 'applications/phid/view/PHUIHandleTagListView.php',
+ 'PHUIHandleView' => 'applications/phid/view/PHUIHandleView.php',
'PHUIHeaderView' => 'view/phui/PHUIHeaderView.php',
'PHUIIconExample' => 'applications/uiexample/examples/PHUIIconExample.php',
'PHUIIconView' => 'view/phui/PHUIIconView.php',
@@ -4423,7 +4425,9 @@
'PHUIFormLayoutView' => 'AphrontView',
'PHUIFormMultiSubmitControl' => 'AphrontFormControl',
'PHUIFormPageView' => 'AphrontView',
+ 'PHUIHandleListView' => 'AphrontTagView',
'PHUIHandleTagListView' => 'AphrontTagView',
+ 'PHUIHandleView' => 'AphrontView',
'PHUIHeaderView' => 'AphrontView',
'PHUIIconExample' => 'PhabricatorUIExample',
'PHUIIconView' => 'AphrontTagView',
diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
--- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
@@ -85,10 +85,6 @@
$phids = array_keys($phids);
$handles = $user->loadHandles($phids);
- // TODO: This is double-loading because we have a separate call to
- // renderHandlesForPHIDs(). Clean this up in the next pass.
- $this->loadHandles($phids);
-
$info_view = null;
if ($parent_task) {
$info_view = new PHUIInfoView();
@@ -100,8 +96,8 @@
->setText(pht('Create Another Subtask')));
$info_view->appendChild(hsprintf(
- 'Created a subtask of <strong>%s</strong>',
- $handles[$parent_task->getPHID()]->renderLink()));
+ 'Created a subtask of <strong>%s</strong>.',
+ $handles->renderHandle($parent_task->getPHID())));
} else if ($workflow == 'create') {
$info_view = new PHUIInfoView();
$info_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE);
@@ -457,7 +453,7 @@
$view->addProperty(
pht('Assigned To'),
$task->getOwnerPHID()
- ? $handles[$task->getOwnerPHID()]->renderLink()
+ ? $handles->renderHandle($task->getOwnerPHID())
: phutil_tag('em', array(), pht('None')));
$view->addProperty(
@@ -466,7 +462,7 @@
$view->addProperty(
pht('Author'),
- $handles[$task->getAuthorPHID()]->renderLink());
+ $handles->renderHandle($task->getAuthorPHID()));
$source = $task->getOriginalEmailSource();
if ($source) {
@@ -504,7 +500,7 @@
->execute();
foreach ($commit_phids as $phid) {
- $revisions_commits[$phid] = $handles[$phid]->renderLink();
+ $revisions_commits[$phid] = $handles->renderHandle($phid);
$revision_phid = key($drev_edges[$phid][$commit_drev]);
$revision_handle = $handles->getHandleIfExists($revision_phid);
if ($revision_handle) {
@@ -520,9 +516,10 @@
foreach ($edge_types as $edge_type => $edge_name) {
if ($edges[$edge_type]) {
+ $edge_handles = $viewer->loadHandles(array_keys($edges[$edge_type]));
$view->addProperty(
$edge_name,
- $this->renderHandlesForPHIDs(array_keys($edges[$edge_type])));
+ $edge_handles->renderList());
}
}
diff --git a/src/applications/phid/handle/pool/PhabricatorHandleList.php b/src/applications/phid/handle/pool/PhabricatorHandleList.php
--- a/src/applications/phid/handle/pool/PhabricatorHandleList.php
+++ b/src/applications/phid/handle/pool/PhabricatorHandleList.php
@@ -26,6 +26,7 @@
private $phids;
private $handles;
private $cursor;
+ private $map;
public function setHandlePool(PhabricatorHandlePool $pool) {
$this->handlePool = $pool;
@@ -71,6 +72,33 @@
}
+/* -( Rendering )---------------------------------------------------------- */
+
+
+ /**
+ * Return a @{class:PHUIHandleListView} which can render the handles in
+ * this list.
+ */
+ public function renderList() {
+ return id(new PHUIHandleListView())
+ ->setHandleList($this);
+ }
+
+
+ /**
+ * Return a @{class:PHUIHandleView} which can render a specific handle.
+ */
+ public function renderHandle($phid) {
+ if (!isset($this[$phid])) {
+ throw new Exception(
+ pht('Trying to render a handle which does not exist!'));
+ }
+
+ return id(new PHUIHandleView())
+ ->setHandleList($this)
+ ->setHandlePHID($phid);
+ }
+
/* -( Iterator )----------------------------------------------------------- */
@@ -99,10 +127,15 @@
public function offsetExists($offset) {
- if ($this->handles === null) {
- $this->loadHandles();
+ // NOTE: We're intentionally not loading handles here so that isset()
+ // checks do not trigger fetches. This gives us better bulk loading
+ // behavior, particularly when invoked through methods like renderHandle().
+
+ if ($this->map === null) {
+ $this->map = array_fill_keys($this->phids, true);
}
- return isset($this->handles[$offset]);
+
+ return isset($this->map[$offset]);
}
public function offsetGet($offset) {
diff --git a/src/applications/phid/view/PHUIHandleListView.php b/src/applications/phid/view/PHUIHandleListView.php
new file mode 100644
--- /dev/null
+++ b/src/applications/phid/view/PHUIHandleListView.php
@@ -0,0 +1,51 @@
+<?php
+
+/**
+ * Convenience class for rendering a list of handles.
+ *
+ * This class simplifies rendering a list of handles and improves loading and
+ * caching semantics in the rendering pipeline by delaying bulk loads until the
+ * last possible moment.
+ */
+final class PHUIHandleListView
+ extends AphrontTagView {
+
+ private $handleList;
+ private $inline;
+
+ public function setHandleList(PhabricatorHandleList $list) {
+ $this->handleList = $list;
+ return $this;
+ }
+
+ public function setInline($inline) {
+ $this->inline = $inline;
+ return $this;
+ }
+
+ public function getInline() {
+ return $this->inline;
+ }
+
+ protected function getTagName() {
+ // TODO: It would be nice to render this with a proper <ul />, at least in
+ // block mode, but don't stir the waters up too much for now.
+ return 'span';
+ }
+
+ protected function getTagContent() {
+ $items = array();
+ foreach ($this->handleList as $handle) {
+ $items[] = $handle->renderLink();
+ }
+
+ if ($this->getInline()) {
+ $items = phutil_implode_html(', ', $items);
+ } else {
+ $items = phutil_implode_html(phutil_tag('br'), $items);
+ }
+
+ return $items;
+ }
+
+}
diff --git a/src/applications/phid/view/PHUIHandleView.php b/src/applications/phid/view/PHUIHandleView.php
new file mode 100644
--- /dev/null
+++ b/src/applications/phid/view/PHUIHandleView.php
@@ -0,0 +1,31 @@
+<?php
+
+/**
+ * Convenience class for rendering a single handle.
+ *
+ * This class simplifies rendering a single handle, and improves loading and
+ * caching semantics in the rendering pipeline by loading data at the last
+ * moment.
+ */
+
+final class PHUIHandleView
+ extends AphrontView {
+
+ private $handleList;
+ private $handlePHID;
+
+ public function setHandleList(PhabricatorHandleList $list) {
+ $this->handleList = $list;
+ return $this;
+ }
+
+ public function setHandlePHID($phid) {
+ $this->handlePHID = $phid;
+ return $this;
+ }
+
+ public function render() {
+ return $this->handleList[$this->handlePHID]->renderLink();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Sun, May 12, 4:28 AM (3 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6268476
Default Alt Text
D12205.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility