From 851aba80ce063d6101f70d3557155a67db0dab4e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 20 Jun 2016 11:49:11 -0700 Subject: [PATCH] Render dropdown metadata earlier Summary: Ref T11179. One issue I'm getting with trying to turn actions into dropdowns is that we currently render this menu very late, which can cause us to try to add more metadata after we start resolving metadata. This won't work right now (and making it work seems unreasonably complicated), so stop doing it and fatal if something tries. (This might make some things fatal but //should// be safe -- anything that fatals should have been broken already.) Test Plan: Browsed around looking for fatals, didn't see any. (This primarily avoids a broken state / fatal in a future diff.) Reviewers: chad Reviewed By: chad Maniphest Tasks: T11179 Differential Revision: https://secure.phabricator.com/D16151 --- .../celerity/CelerityStaticResourceResponse.php | 10 ++++++++++ src/view/layout/PhabricatorActionListView.php | 6 ++++++ src/view/phui/PHUIButtonView.php | 5 +---- src/view/phui/PHUIListItemView.php | 5 +---- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/applications/celerity/CelerityStaticResourceResponse.php b/src/applications/celerity/CelerityStaticResourceResponse.php index f81b8d5957..7db6c2741b 100644 --- a/src/applications/celerity/CelerityStaticResourceResponse.php +++ b/src/applications/celerity/CelerityStaticResourceResponse.php @@ -13,6 +13,7 @@ final class CelerityStaticResourceResponse extends Phobject { private $packaged; private $metadata = array(); private $metadataBlock = 0; + private $metadataLocked; private $behaviors = array(); private $hasRendered = array(); private $postprocessorKey; @@ -24,6 +25,13 @@ final class CelerityStaticResourceResponse extends Phobject { } public function addMetadata($metadata) { + if ($this->metadataLocked) { + throw new Exception( + pht( + 'Attempting to add more metadata after metadata has been '. + 'locked.')); + } + $id = count($this->metadata); $this->metadata[$id] = $metadata; return $this->metadataBlock.'_'.$id; @@ -189,6 +197,8 @@ final class CelerityStaticResourceResponse extends Phobject { } public function renderHTMLFooter() { + $this->metadataLocked = true; + $data = array(); if ($this->metadata) { $json_metadata = AphrontResponse::encodeJSONForHTTPResponse( diff --git a/src/view/layout/PhabricatorActionListView.php b/src/view/layout/PhabricatorActionListView.php index 4965f02793..5c3facbf42 100644 --- a/src/view/layout/PhabricatorActionListView.php +++ b/src/view/layout/PhabricatorActionListView.php @@ -53,5 +53,11 @@ final class PhabricatorActionListView extends AphrontView { $actions); } + public function getDropdownMenuMetadata() { + return array( + 'items' => (string)hsprintf('%s', $this), + ); + } + } diff --git a/src/view/phui/PHUIButtonView.php b/src/view/phui/PHUIButtonView.php index 9b9d4f0293..f9266a170a 100644 --- a/src/view/phui/PHUIButtonView.php +++ b/src/view/phui/PHUIButtonView.php @@ -105,10 +105,7 @@ final class PHUIButtonView extends AphrontTagView { Javelin::initBehavior('phui-dropdown-menu'); $this->addSigil('phui-dropdown-menu'); - $this->setMetadata( - array( - 'items' => $actions, - )); + $this->setMetadata($actions->getDropdownMenuMetadata()); return $this; } diff --git a/src/view/phui/PHUIListItemView.php b/src/view/phui/PHUIListItemView.php index 33c511c9c0..4af02e572e 100644 --- a/src/view/phui/PHUIListItemView.php +++ b/src/view/phui/PHUIListItemView.php @@ -43,10 +43,7 @@ final class PHUIListItemView extends AphrontTagView { Javelin::initBehavior('phui-dropdown-menu'); $this->addSigil('phui-dropdown-menu'); - $this->setMetadata( - array( - 'items' => $actions, - )); + $this->setMetadata($actions->getDropdownMenuMetadata()); return $this; }