@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Provide hasChildren() to replace isEmptyContent()

Summary:
Fixes T3698. Sometimes views need to render differently depending on whether they contain content or not. The existing approach for this is `isEmptyContent()`, which doesn't work well and is sort of hacky (it implies double-rendering content, which is not always free or side-effect free).

Instead, provide a test for an element without children. This test is powerful enough to catch the easy cases of `null`, etc., and just do the expected thing, but will not catch a View which is reduced upon rendering. Since this is rare and we have no actual need for it today, just accept that as a limitation.

Test Plan:
Viewed Timeline and Feed UI examples. Viewed Feed (feed), Pholio (timelineview), and Differential (old transactionview).

{F53915}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T3698

Differential Revision: https://secure.phabricator.com/D6718

+128 -25
+2
src/__phutil_library_map__.php
··· 775 775 'PhabricatorAllCapsTranslation' => 'infrastructure/internationalization/PhabricatorAllCapsTranslation.php', 776 776 'PhabricatorAnchorView' => 'view/layout/PhabricatorAnchorView.php', 777 777 'PhabricatorAphrontBarExample' => 'applications/uiexample/examples/PhabricatorAphrontBarExample.php', 778 + 'PhabricatorAphrontViewTestCase' => 'view/__tests__/PhabricatorAphrontViewTestCase.php', 778 779 'PhabricatorApplication' => 'applications/base/PhabricatorApplication.php', 779 780 'PhabricatorApplicationApplications' => 'applications/meta/application/PhabricatorApplicationApplications.php', 780 781 'PhabricatorApplicationAudit' => 'applications/audit/application/PhabricatorApplicationAudit.php', ··· 2804 2805 'PhabricatorAllCapsTranslation' => 'PhabricatorTranslation', 2805 2806 'PhabricatorAnchorView' => 'AphrontView', 2806 2807 'PhabricatorAphrontBarExample' => 'PhabricatorUIExample', 2808 + 'PhabricatorAphrontViewTestCase' => 'PhabricatorTestCase', 2807 2809 'PhabricatorApplicationApplications' => 'PhabricatorApplication', 2808 2810 'PhabricatorApplicationAudit' => 'PhabricatorApplication', 2809 2811 'PhabricatorApplicationAuth' => 'PhabricatorApplication',
+1 -1
src/view/AphrontNullView.php
··· 3 3 final class AphrontNullView extends AphrontView { 4 4 5 5 public function render() { 6 - return phutil_implode_html('', $this->renderChildren()); 6 + return $this->renderChildren(); 7 7 } 8 8 9 9 }
+94 -15
src/view/AphrontView.php
··· 1 1 <?php 2 2 3 + /** 4 + * @task children Managing Children 5 + */ 3 6 abstract class AphrontView extends Phobject 4 7 implements PhutilSafeHTMLProducerInterface { 5 8 6 9 protected $user; 7 10 protected $children = array(); 8 11 12 + 13 + /* -( Configuration )------------------------------------------------------ */ 14 + 15 + 16 + /** 17 + * @task config 18 + */ 9 19 public function setUser(PhabricatorUser $user) { 10 20 $this->user = $user; 11 21 return $this; 12 22 } 13 23 24 + 25 + /** 26 + * @task config 27 + */ 14 28 protected function getUser() { 15 29 return $this->user; 16 30 } 17 31 32 + 33 + /* -( Managing Children )-------------------------------------------------- */ 34 + 35 + 36 + /** 37 + * Test if this View accepts children. 38 + * 39 + * By default, views accept children, but subclases may override this method 40 + * to prevent children from being appended. Doing so will cause 41 + * @{method:appendChild} to throw exceptions instead of appending children. 42 + * 43 + * @return bool True if the View should accept children. 44 + * @task children 45 + */ 18 46 protected function canAppendChild() { 19 47 return true; 20 48 } 21 49 50 + 51 + /** 52 + * Append a child to the list of children. 53 + * 54 + * This method will only work if the view supports children, which is 55 + * determined by @{method:canAppendChild}. 56 + * 57 + * @param wild Something renderable. 58 + * @return this 59 + */ 22 60 final public function appendChild($child) { 23 61 if (!$this->canAppendChild()) { 24 62 $class = get_class($this); 25 63 throw new Exception( 26 64 pht("View '%s' does not support children.", $class)); 27 65 } 66 + 28 67 $this->children[] = $child; 68 + 29 69 return $this; 30 70 } 31 71 72 + 73 + /** 74 + * Produce children for rendering. 75 + * 76 + * Historically, this method reduced children to a string representation, 77 + * but it no longer does. 78 + * 79 + * @return wild Renderable children. 80 + * @task 81 + */ 32 82 final protected function renderChildren() { 33 83 return $this->children; 34 84 } 35 85 86 + 36 87 /** 37 - * @deprecated 88 + * Test if an element has no children. 89 + * 90 + * @return bool True if this element has children. 91 + * @task children 38 92 */ 39 - final protected function renderSingleView($child) { 40 - phutil_deprecated( 41 - 'AphrontView->renderSingleView()', 42 - "This method no longer does anything; it can be removed and replaced ". 43 - "with its arguments."); 44 - return $child; 93 + final public function hasChildren() { 94 + if ($this->children) { 95 + $this->children = $this->reduceChildren($this->children); 96 + } 97 + return (bool)$this->children; 45 98 } 46 99 47 - final protected function isEmptyContent($content) { 48 - if (is_array($content)) { 49 - foreach ($content as $element) { 50 - if (!$this->isEmptyContent($element)) { 51 - return false; 100 + 101 + /** 102 + * Reduce effectively-empty lists of children to be actually empty. This 103 + * recursively removes `null`, `''`, and `array()` from the list of children 104 + * so that @{method:hasChildren} can more effectively align with expectations. 105 + * 106 + * NOTE: Because View children are not rendered, a View which renders down 107 + * to nothing will not be reduced by this method. 108 + * 109 + * @param list<wild> Renderable children. 110 + * @return list<wild> Reduced list of children. 111 + * @task children 112 + */ 113 + private function reduceChildren(array $children) { 114 + foreach ($children as $key => $child) { 115 + if ($child === null) { 116 + unset($children[$key]); 117 + } else if ($child === '') { 118 + unset($children[$key]); 119 + } else if (is_array($child)) { 120 + $child = $this->reduceChildren($child); 121 + if ($child) { 122 + $children[$key] = $child; 123 + } else { 124 + unset($children[$key]); 52 125 } 53 126 } 54 - return true; 55 - } else { 56 - return !strlen((string)$content); 57 127 } 128 + return $children; 58 129 } 59 130 131 + 132 + /* -( Rendering )---------------------------------------------------------- */ 133 + 134 + 60 135 abstract public function render(); 136 + 137 + 138 + /* -( PhutilSafeHTMLProducerInterface )------------------------------------ */ 139 + 61 140 62 141 public function producePhutilSafeHTML() { 63 142 return $this->render();
+25
src/view/__tests__/PhabricatorAphrontViewTestCase.php
··· 1 + <?php 2 + 3 + final class PhabricatorAphrontViewTestCase extends PhabricatorTestCase { 4 + 5 + public function testHasChildren() { 6 + $view = new AphrontNullView(); 7 + $this->assertEqual(false, $view->hasChildren()); 8 + 9 + $values = array( 10 + null, 11 + '', 12 + array(), 13 + array(null, ''), 14 + ); 15 + 16 + foreach ($values as $value) { 17 + $view->appendChild($value); 18 + $this->assertEqual(false, $view->hasChildren()); 19 + } 20 + 21 + $view->appendChild("!"); 22 + $this->assertEqual(true, $view->hasChildren()); 23 + } 24 + 25 + }
+3 -5
src/view/layout/PhabricatorTimelineEventView.php
··· 100 100 } 101 101 102 102 public function render() { 103 - $content = $this->renderChildren(); 104 - 105 103 $title = $this->title; 106 - if (($title === null) && $this->isEmptyContent($content)) { 104 + if (($title === null) && !$this->hasChildren()) { 107 105 $title = ''; 108 106 } 109 107 ··· 163 161 $classes = array(); 164 162 $classes[] = 'phabricator-timeline-event-view'; 165 163 $classes[] = 'phabricator-timeline-border'; 166 - if (!$this->isEmptyContent($content)) { 164 + if ($this->hasChildren()) { 167 165 $classes[] = 'phabricator-timeline-major-event'; 168 166 $content = phutil_tag( 169 167 'div', ··· 182 180 array( 183 181 'class' => 'phabricator-timeline-core-content', 184 182 ), 185 - $content), 183 + $this->renderChildren()), 186 184 ))); 187 185 $content = array($image, $wedge, $content); 188 186 } else {
+2 -3
src/view/layout/PhabricatorTransactionView.php
··· 136 136 } 137 137 138 138 private function renderTransactionContent() { 139 - $content = $this->renderChildren(); 140 - if ($this->isEmptyContent($content)) { 139 + if (!$this->hasChildren()) { 141 140 return null; 142 141 } 143 142 return phutil_tag( 144 143 'div', 145 144 array('class' => 'phabricator-transaction-content'), 146 - $content); 145 + $this->renderChildren()); 147 146 } 148 147 149 148 }
+1 -1
src/view/phui/PHUIFeedStoryView.php
··· 124 124 125 125 require_celerity_resource('phui-feed-story-css'); 126 126 Javelin::initBehavior('phabricator-hovercards'); 127 - $oneline = $this->isEmptyContent($this->renderChildren()); 127 + $oneline = !$this->hasChildren(); 128 128 129 129 $body = null; 130 130 $foot = null;