From bdc517485cb24d40bb2e700a49514a5142d0a15d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Dec 2015 11:20:18 -0800 Subject: [PATCH] Modernize Hovercard implementation Summary: Ref T8980. Move away from events to EngineExtensions. This also simplifies hovercards a bit: - Removes tasks from revision cards. - Removes blockers/blocked from task cards. - Removes "Send Message" from user cards. These mostly felt cluttery to me. Open to arguments to retain them. I think we can make better use of the space, though (e.g., flags, projects + board columns). Test Plan: - Viewed people, task, revision, commit and project hovercards. {F1043256} {F1043257} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8980 Differential Revision: https://secure.phabricator.com/D14878 --- resources/celerity/map.php | 20 ++-- src/__phutil_library_map__.php | 22 +++-- .../PhabricatorConpherenceApplication.php | 6 -- .../ConpherenceHovercardEventListener.php | 41 -------- .../PhabricatorDifferentialApplication.php | 1 - .../DifferentialHovercardEngineExtension.php | 77 +++++++++++++++ .../DifferentialHovercardEventListener.php | 71 -------------- .../PhabricatorDiffusionApplication.php | 6 -- .../DiffusionHovercardEngineExtension.php | 53 +++++++++++ .../DiffusionHovercardEventListener.php | 75 --------------- .../PhabricatorManiphestApplication.php | 6 -- .../ManiphestHovercardEngineExtension.php | 47 ++++++++++ .../event/ManiphestHovercardEventListener.php | 93 ------------------- .../PhabricatorPeopleApplication.php | 6 -- ...ricatorPeopleHovercardEngineExtension.php} | 64 +++++++------ .../PhabricatorSearchApplication.php | 2 +- .../PhabricatorSearchHovercardController.php | 82 ++++++++++------ .../PhabricatorHovercardEngineExtension.php | 60 ++++++++++++ ...bricatorHovercardEngineExtensionModule.php | 55 +++++++++++ .../events/constant/PhabricatorEventType.php | 2 - webroot/rsrc/js/core/Hovercard.js | 2 +- 21 files changed, 407 insertions(+), 384 deletions(-) delete mode 100644 src/applications/conpherence/events/ConpherenceHovercardEventListener.php create mode 100644 src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php delete mode 100644 src/applications/differential/event/DifferentialHovercardEventListener.php create mode 100644 src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php delete mode 100644 src/applications/diffusion/events/DiffusionHovercardEventListener.php create mode 100644 src/applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php delete mode 100644 src/applications/maniphest/event/ManiphestHovercardEventListener.php rename src/applications/people/{event/PhabricatorPeopleHovercardEventListener.php => engineextension/PhabricatorPeopleHovercardEngineExtension.php} (67%) create mode 100644 src/applications/search/engineextension/PhabricatorHovercardEngineExtension.php create mode 100644 src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 6eed658f34..a5a10ef67a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'a419cf4b', - 'core.pkg.js' => 'cf262309', + 'core.pkg.js' => 'b826f522', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '2de124c9', 'differential.pkg.js' => '64e69521', @@ -452,7 +452,7 @@ return array( 'rsrc/js/core/DragAndDropFileUpload.js' => 'ad10aeac', 'rsrc/js/core/DraggableList.js' => 'a16ec1c6', 'rsrc/js/core/FileUpload.js' => '477359c8', - 'rsrc/js/core/Hovercard.js' => '14ac66f5', + 'rsrc/js/core/Hovercard.js' => '6914d0dd', 'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2', 'rsrc/js/core/KeyboardShortcutManager.js' => 'c1700f6f', 'rsrc/js/core/MultirowRowManager.js' => 'b5d57730', @@ -747,7 +747,7 @@ return array( 'phabricator-file-upload' => '477359c8', 'phabricator-filetree-view-css' => 'fccf9f82', 'phabricator-flag-css' => '5337623f', - 'phabricator-hovercard' => '14ac66f5', + 'phabricator-hovercard' => '6914d0dd', 'phabricator-hovercard-view-css' => '1239cd52', 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => 'c1700f6f', @@ -935,13 +935,6 @@ return array( 'javelin-dom', 'javelin-history', ), - '14ac66f5' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-vector', - 'javelin-request', - 'javelin-uri', - ), '1ad0a787' => array( 'javelin-install', 'javelin-reactor', @@ -1311,6 +1304,13 @@ return array( '6882e80a' => array( 'javelin-dom', ), + '6914d0dd' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-vector', + 'javelin-request', + 'javelin-uri', + ), '69adf288' => array( 'javelin-install', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index dc6b644d4a..51f9aa93b2 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -262,7 +262,6 @@ phutil_register_library_map(array( 'ConpherenceEditor' => 'applications/conpherence/editor/ConpherenceEditor.php', 'ConpherenceFormDragAndDropUploadControl' => 'applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php', 'ConpherenceFulltextQuery' => 'applications/conpherence/query/ConpherenceFulltextQuery.php', - 'ConpherenceHovercardEventListener' => 'applications/conpherence/events/ConpherenceHovercardEventListener.php', 'ConpherenceImageData' => 'applications/conpherence/constants/ConpherenceImageData.php', 'ConpherenceIndex' => 'applications/conpherence/storage/ConpherenceIndex.php', 'ConpherenceLayoutView' => 'applications/conpherence/view/ConpherenceLayoutView.php', @@ -420,7 +419,7 @@ phutil_register_library_map(array( 'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php', 'DifferentialHostedGitLandingStrategy' => 'applications/differential/landing/DifferentialHostedGitLandingStrategy.php', 'DifferentialHostedMercurialLandingStrategy' => 'applications/differential/landing/DifferentialHostedMercurialLandingStrategy.php', - 'DifferentialHovercardEventListener' => 'applications/differential/event/DifferentialHovercardEventListener.php', + 'DifferentialHovercardEngineExtension' => 'applications/differential/engineextension/DifferentialHovercardEngineExtension.php', 'DifferentialHunk' => 'applications/differential/storage/DifferentialHunk.php', 'DifferentialHunkParser' => 'applications/differential/parser/DifferentialHunkParser.php', 'DifferentialHunkParserTestCase' => 'applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php', @@ -620,7 +619,7 @@ phutil_register_library_map(array( 'DiffusionHistoryController' => 'applications/diffusion/controller/DiffusionHistoryController.php', 'DiffusionHistoryQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php', 'DiffusionHistoryTableView' => 'applications/diffusion/view/DiffusionHistoryTableView.php', - 'DiffusionHovercardEventListener' => 'applications/diffusion/events/DiffusionHovercardEventListener.php', + 'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php', 'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php', 'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php', 'DiffusionLastModifiedController' => 'applications/diffusion/controller/DiffusionLastModifiedController.php', @@ -1292,7 +1291,7 @@ phutil_register_library_map(array( 'ManiphestExcelFormatTestCase' => 'applications/maniphest/export/__tests__/ManiphestExcelFormatTestCase.php', 'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php', - 'ManiphestHovercardEventListener' => 'applications/maniphest/event/ManiphestHovercardEventListener.php', + 'ManiphestHovercardEngineExtension' => 'applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php', 'ManiphestInfoConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php', 'ManiphestNameIndex' => 'applications/maniphest/storage/ManiphestNameIndex.php', 'ManiphestPriorityConfigOptionType' => 'applications/maniphest/config/ManiphestPriorityConfigOptionType.php', @@ -2370,6 +2369,8 @@ phutil_register_library_map(array( 'PhabricatorHomeMainController' => 'applications/home/controller/PhabricatorHomeMainController.php', 'PhabricatorHomePreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php', 'PhabricatorHomeQuickCreateController' => 'applications/home/controller/PhabricatorHomeQuickCreateController.php', + 'PhabricatorHovercardEngineExtension' => 'applications/search/engineextension/PhabricatorHovercardEngineExtension.php', + 'PhabricatorHovercardEngineExtensionModule' => 'applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php', 'PhabricatorHovercardUIExample' => 'applications/uiexample/examples/PhabricatorHovercardUIExample.php', 'PhabricatorHovercardView' => 'view/widget/hovercard/PhabricatorHovercardView.php', 'PhabricatorHunksManagementMigrateWorkflow' => 'applications/differential/management/PhabricatorHunksManagementMigrateWorkflow.php', @@ -2711,7 +2712,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleDisableController' => 'applications/people/controller/PhabricatorPeopleDisableController.php', 'PhabricatorPeopleEmpowerController' => 'applications/people/controller/PhabricatorPeopleEmpowerController.php', 'PhabricatorPeopleExternalPHIDType' => 'applications/people/phid/PhabricatorPeopleExternalPHIDType.php', - 'PhabricatorPeopleHovercardEventListener' => 'applications/people/event/PhabricatorPeopleHovercardEventListener.php', + 'PhabricatorPeopleHovercardEngineExtension' => 'applications/people/engineextension/PhabricatorPeopleHovercardEngineExtension.php', 'PhabricatorPeopleInviteController' => 'applications/people/controller/PhabricatorPeopleInviteController.php', 'PhabricatorPeopleInviteListController' => 'applications/people/controller/PhabricatorPeopleInviteListController.php', 'PhabricatorPeopleInviteSendController' => 'applications/people/controller/PhabricatorPeopleInviteSendController.php', @@ -4172,7 +4173,6 @@ phutil_register_library_map(array( 'ConpherenceEditor' => 'PhabricatorApplicationTransactionEditor', 'ConpherenceFormDragAndDropUploadControl' => 'AphrontFormControl', 'ConpherenceFulltextQuery' => 'PhabricatorOffsetPagedQuery', - 'ConpherenceHovercardEventListener' => 'PhabricatorEventListener', 'ConpherenceImageData' => 'ConpherenceConstants', 'ConpherenceIndex' => 'ConpherenceDAO', 'ConpherenceLayoutView' => 'AphrontView', @@ -4347,7 +4347,7 @@ phutil_register_library_map(array( 'DifferentialHostField' => 'DifferentialCustomField', 'DifferentialHostedGitLandingStrategy' => 'DifferentialLandingStrategy', 'DifferentialHostedMercurialLandingStrategy' => 'DifferentialLandingStrategy', - 'DifferentialHovercardEventListener' => 'PhabricatorEventListener', + 'DifferentialHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'DifferentialHunk' => array( 'DifferentialDAO', 'PhabricatorPolicyInterface', @@ -4568,7 +4568,7 @@ phutil_register_library_map(array( 'DiffusionHistoryController' => 'DiffusionController', 'DiffusionHistoryQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionHistoryTableView' => 'DiffusionView', - 'DiffusionHovercardEventListener' => 'PhabricatorEventListener', + 'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController', 'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController', 'DiffusionLastModifiedController' => 'DiffusionController', @@ -5361,7 +5361,7 @@ phutil_register_library_map(array( 'ManiphestExcelFormatTestCase' => 'PhabricatorTestCase', 'ManiphestExportController' => 'ManiphestController', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod', - 'ManiphestHovercardEventListener' => 'PhabricatorEventListener', + 'ManiphestHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'ManiphestInfoConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestNameIndex' => 'ManiphestDAO', 'ManiphestPriorityConfigOptionType' => 'PhabricatorConfigJSONOptionType', @@ -6624,6 +6624,8 @@ phutil_register_library_map(array( 'PhabricatorHomeMainController' => 'PhabricatorHomeController', 'PhabricatorHomePreferencesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorHomeQuickCreateController' => 'PhabricatorHomeController', + 'PhabricatorHovercardEngineExtension' => 'Phobject', + 'PhabricatorHovercardEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorHovercardUIExample' => 'PhabricatorUIExample', 'PhabricatorHovercardView' => 'AphrontView', 'PhabricatorHunksManagementMigrateWorkflow' => 'PhabricatorHunksManagementWorkflow', @@ -7010,7 +7012,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleDisableController' => 'PhabricatorPeopleController', 'PhabricatorPeopleEmpowerController' => 'PhabricatorPeopleController', 'PhabricatorPeopleExternalPHIDType' => 'PhabricatorPHIDType', - 'PhabricatorPeopleHovercardEventListener' => 'PhabricatorEventListener', + 'PhabricatorPeopleHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'PhabricatorPeopleInviteController' => 'PhabricatorPeopleController', 'PhabricatorPeopleInviteListController' => 'PhabricatorPeopleInviteController', 'PhabricatorPeopleInviteSendController' => 'PhabricatorPeopleInviteController', diff --git a/src/applications/conpherence/application/PhabricatorConpherenceApplication.php b/src/applications/conpherence/application/PhabricatorConpherenceApplication.php index 852daa8bee..7708e6a539 100644 --- a/src/applications/conpherence/application/PhabricatorConpherenceApplication.php +++ b/src/applications/conpherence/application/PhabricatorConpherenceApplication.php @@ -28,12 +28,6 @@ final class PhabricatorConpherenceApplication extends PhabricatorApplication { ); } - public function getEventListeners() { - return array( - new ConpherenceHovercardEventListener(), - ); - } - public function getRoutes() { return array( '/Z(?P[1-9]\d*)' => 'ConpherenceViewController', diff --git a/src/applications/conpherence/events/ConpherenceHovercardEventListener.php b/src/applications/conpherence/events/ConpherenceHovercardEventListener.php deleted file mode 100644 index dcf4f08294..0000000000 --- a/src/applications/conpherence/events/ConpherenceHovercardEventListener.php +++ /dev/null @@ -1,41 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD: - $this->handleHovercardEvent($event); - break; - } - } - - private function handleHovercardEvent($event) { - $hovercard = $event->getValue('hovercard'); - $user = $event->getValue('object'); - - if (!($user instanceof PhabricatorUser)) { - return; - } - - $conpherence_uri = id(new PhutilURI('/conpherence/new/')) - ->setQueryParam('participant', $user->getPHID()); - $name = pht('Send a Message'); - $hovercard->addAction($name, $conpherence_uri, true); - - $event->setValue('hovercard', $hovercard); - } - -} diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php index e8113dc3bd..a3074d3e31 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -44,7 +44,6 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { public function getEventListeners() { return array( new DifferentialActionMenuEventListener(), - new DifferentialHovercardEventListener(), new DifferentialLandingActionMenuEventListener(), ); } diff --git a/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php b/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php new file mode 100644 index 0000000000..325e99f860 --- /dev/null +++ b/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php @@ -0,0 +1,77 @@ +getViewer(); + $phids = mpull($objects, 'getPHID'); + + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs($phids) + ->needReviewerStatus(true) + ->execute(); + $revisions = mpull($revisions, null, 'getPHID'); + + return array( + 'revisions' => $revisions, + ); + } + + public function renderHovercard( + PhabricatorHovercardView $hovercard, + PhabricatorObjectHandle $handle, + $object, + $data) { + + $viewer = $this->getViewer(); + + $revision = idx($data['revisions'], $object->getPHID()); + if (!$revision) { + return; + } + + $hovercard->setTitle('D'.$revision->getID()); + $hovercard->setDetail($revision->getTitle()); + + $hovercard->addField( + pht('Author'), + $viewer->renderHandle($revision->getAuthorPHID())); + + $reviewer_phids = $revision->getReviewerStatus(); + $reviewer_phids = mpull($reviewer_phids, 'getReviewerPHID'); + + $hovercard->addField( + pht('Reviewers'), + $viewer->renderHandleList($reviewer_phids)->setAsInline(true)); + + $summary = $revision->getSummary(); + if (strlen($summary)) { + $summary = id(new PhutilUTF8StringTruncator()) + ->setMaximumGlyphs(120) + ->truncateString($summary); + + $hovercard->addField(pht('Summary'), $summary); + } + + $tag = DifferentialRevisionDetailView::renderTagForRevision($revision); + $hovercard->addTag($tag); + } + +} diff --git a/src/applications/differential/event/DifferentialHovercardEventListener.php b/src/applications/differential/event/DifferentialHovercardEventListener.php deleted file mode 100644 index 5c53a533b1..0000000000 --- a/src/applications/differential/event/DifferentialHovercardEventListener.php +++ /dev/null @@ -1,71 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD: - $this->handleHovercardEvent($event); - break; - } - } - - private function handleHovercardEvent($event) { - $viewer = $event->getUser(); - $hovercard = $event->getValue('hovercard'); - $object_handle = $event->getValue('handle'); - $phid = $object_handle->getPHID(); - $rev = $event->getValue('object'); - - if (!($rev instanceof DifferentialRevision)) { - return; - } - - $rev->loadRelationships(); - $reviewer_phids = $rev->getReviewers(); - $e_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; - $edge_query = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(array($phid)) - ->withEdgeTypes( - array( - $e_task, - )); - $edge_query->execute(); - $tasks = $edge_query->getDestinationPHIDs(); - - $hovercard->setTitle('D'.$rev->getID()); - $hovercard->setDetail($rev->getTitle()); - - $hovercard->addField( - pht('Author'), - $viewer->renderHandle($rev->getAuthorPHID())); - - $hovercard->addField( - pht('Reviewers'), - $viewer->renderHandleList($reviewer_phids)->setAsInline(true)); - - if ($tasks) { - $hovercard->addField( - pht('Tasks'), - $viewer->renderHandleList($tasks)->setAsInline(true)); - } - - if ($rev->getSummary()) { - $hovercard->addField(pht('Summary'), - id(new PhutilUTF8StringTruncator()) - ->setMaximumGlyphs(120) - ->truncateString($rev->getSummary())); - } - - $hovercard->addTag( - DifferentialRevisionDetailView::renderTagForRevision($rev)); - - $event->setValue('hovercard', $hovercard); - } - -} diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php index 2e3d2a4110..cbbfff8763 100644 --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -37,12 +37,6 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { ); } - public function getEventListeners() { - return array( - new DiffusionHovercardEventListener(), - ); - } - public function getRemarkupRules() { return array( new DiffusionCommitRemarkupRule(), diff --git a/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php b/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php new file mode 100644 index 0000000000..a8193e569f --- /dev/null +++ b/src/applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php @@ -0,0 +1,53 @@ +getViewer(); + + $author_phid = $commit->getAuthorPHID(); + if ($author_phid) { + $author = $viewer->loadHandle($author)->renderLink(); + } else { + $commit_data = $commit->loadCommitData(); + $author = phutil_tag('em', array(), $commit_data->getAuthorName()); + } + + $hovercard->setTitle($handle->getName()); + $hovercard->setDetail($commit->getSummary()); + + $hovercard->addField(pht('Author'), $author); + $hovercard->addField(pht('Date'), + phabricator_date($commit->getEpoch(), $viewer)); + + if ($commit->getAuditStatus() != + PhabricatorAuditCommitStatusConstants::NONE) { + + $hovercard->addField(pht('Audit Status'), + PhabricatorAuditCommitStatusConstants::getStatusName( + $commit->getAuditStatus())); + } + } + +} diff --git a/src/applications/diffusion/events/DiffusionHovercardEventListener.php b/src/applications/diffusion/events/DiffusionHovercardEventListener.php deleted file mode 100644 index 4a979dbf4a..0000000000 --- a/src/applications/diffusion/events/DiffusionHovercardEventListener.php +++ /dev/null @@ -1,75 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD: - $this->handleHovercardEvent($event); - break; - } - } - - private function handleHovercardEvent($event) { - $viewer = $event->getUser(); - $hovercard = $event->getValue('hovercard'); - $object_handle = $event->getValue('handle'); - $commit = $event->getValue('object'); - - if (!($commit instanceof PhabricatorRepositoryCommit)) { - return; - } - - $commit_data = $commit->loadCommitData(); - - $revision = PhabricatorEdgeQuery::loadDestinationPHIDs( - $commit->getPHID(), - DiffusionCommitHasRevisionEdgeType::EDGECONST); - $revision = reset($revision); - - $author = $commit->getAuthorPHID(); - - $phids = array_filter(array( - $revision, - $author, - )); - - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($viewer) - ->withPHIDs($phids) - ->execute(); - - if ($author) { - $author = $handles[$author]->renderLink(); - } else { - $author = phutil_tag('em', array(), $commit_data->getAuthorName()); - } - - $hovercard->setTitle($object_handle->getName()); - $hovercard->setDetail($commit->getSummary()); - - $hovercard->addField(pht('Author'), $author); - $hovercard->addField(pht('Date'), - phabricator_date($commit->getEpoch(), $viewer)); - - if ($commit->getAuditStatus() != - PhabricatorAuditCommitStatusConstants::NONE) { - - $hovercard->addField(pht('Audit Status'), - PhabricatorAuditCommitStatusConstants::getStatusName( - $commit->getAuditStatus())); - } - - if ($revision) { - $rev_handle = $handles[$revision]; - $hovercard->addField(pht('Revision'), $rev_handle->renderLink()); - } - - $event->setValue('hovercard', $hovercard); - } - -} diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php index a9c5ddcd01..c0d0a4d7ef 100644 --- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php +++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php @@ -36,12 +36,6 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication { ); } - public function getEventListeners() { - return array( - new ManiphestHovercardEventListener(), - ); - } - public function getRemarkupRules() { return array( new ManiphestRemarkupRule(), diff --git a/src/applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php b/src/applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php new file mode 100644 index 0000000000..5215232a7d --- /dev/null +++ b/src/applications/maniphest/engineextension/ManiphestHovercardEngineExtension.php @@ -0,0 +1,47 @@ +getViewer(); + + $hovercard + ->setTitle($task->getMonogram()) + ->setDetail($task->getTitle()); + + $owner_phid = $task->getOwnerPHID(); + if ($owner_phid) { + $owner = $viewer->renderHandle($owner_phid); + } else { + $owner = phutil_tag('em', array(), pht('None')); + } + $hovercard->addField(pht('Assigned To'), $owner); + + $hovercard->addField( + pht('Priority'), + ManiphestTaskPriority::getTaskPriorityName($task->getPriority())); + + $hovercard->addTag(ManiphestView::renderTagForTask($task)); + } + +} diff --git a/src/applications/maniphest/event/ManiphestHovercardEventListener.php b/src/applications/maniphest/event/ManiphestHovercardEventListener.php deleted file mode 100644 index 5615b78d04..0000000000 --- a/src/applications/maniphest/event/ManiphestHovercardEventListener.php +++ /dev/null @@ -1,93 +0,0 @@ -listen(PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD); - } - - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD: - $this->handleHovercardEvent($event); - break; - } - } - - private function handleHovercardEvent(PhutilEvent $event) { - $viewer = $event->getUser(); - $hovercard = $event->getValue('hovercard'); - $handle = $event->getValue('handle'); - $phid = $handle->getPHID(); - $task = $event->getValue('object'); - - if (!($task instanceof ManiphestTask)) { - return; - } - - $e_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; - // Fun with "Unbeta Pholio", hua hua - $e_dep_on = ManiphestTaskDependsOnTaskEdgeType::EDGECONST; - $e_dep_by = ManiphestTaskDependedOnByTaskEdgeType::EDGECONST; - - $edge_query = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(array($phid)) - ->withEdgeTypes( - array( - $e_project, - $e_dep_on, - $e_dep_by, - )); - $edges = idx($edge_query->execute(), $phid); - $edge_phids = $edge_query->getDestinationPHIDs(); - - $owner_phid = $task->getOwnerPHID(); - - $hovercard - ->setTitle(pht('T%d', $task->getID())) - ->setDetail($task->getTitle()); - - if ($owner_phid) { - $owner = $viewer->renderHandle($owner_phid); - } else { - $owner = phutil_tag('em', array(), pht('None')); - } - $hovercard->addField(pht('Assigned To'), $owner); - $hovercard->addField( - pht('Priority'), - ManiphestTaskPriority::getTaskPriorityName($task->getPriority())); - - if ($edge_phids) { - $edge_types = array( - $e_project => pht('Projects'), - $e_dep_by => pht('Blocks'), - $e_dep_on => pht('Blocked By'), - ); - - $max_count = 6; - foreach ($edge_types as $edge_type => $edge_name) { - if ($edges[$edge_type]) { - // TODO: This can be made more sophisticated. We still load all - // edges into memory. Only load the ones we need. - $edge_overflow = array(); - if (count($edges[$edge_type]) > $max_count) { - $edges[$edge_type] = array_slice($edges[$edge_type], 0, 6, true); - $edge_overflow = ', ...'; - } - - $hovercard->addField( - $edge_name, - array( - $viewer->renderHandleList(array_keys($edges[$edge_type])), - $edge_overflow, - )); - } - } - } - - $hovercard->addTag(ManiphestView::renderTagForTask($task)); - - $event->setValue('hovercard', $hovercard); - } - -} diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index 9ab8a61bfd..d1cb552e18 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -34,12 +34,6 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { return false; } - public function getEventListeners() { - return array( - new PhabricatorPeopleHovercardEventListener(), - ); - } - public function getRoutes() { return array( '/people/' => array( diff --git a/src/applications/people/event/PhabricatorPeopleHovercardEventListener.php b/src/applications/people/engineextension/PhabricatorPeopleHovercardEngineExtension.php similarity index 67% rename from src/applications/people/event/PhabricatorPeopleHovercardEventListener.php rename to src/applications/people/engineextension/PhabricatorPeopleHovercardEngineExtension.php index 6c8efadd13..8ce8c25a49 100644 --- a/src/applications/people/event/PhabricatorPeopleHovercardEventListener.php +++ b/src/applications/people/engineextension/PhabricatorPeopleHovercardEngineExtension.php @@ -1,45 +1,58 @@ listen(PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD); + const EXTENSIONKEY = 'people'; + + public function isExtensionEnabled() { + return true; } - public function handleEvent(PhutilEvent $event) { - switch ($event->getType()) { - case PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD: - $this->handleHovercardEvent($event); - break; - } + public function getExtensionName() { + return pht('User Accounts'); } - private function handleHovercardEvent($event) { - $viewer = $event->getUser(); - $hovercard = $event->getValue('hovercard'); - $object_handle = $event->getValue('handle'); - $phid = $object_handle->getPHID(); - $user = $event->getValue('object'); + public function canRenderObjectHovercard($object) { + return ($object instanceof PhabricatorUser); + } - if (!($user instanceof PhabricatorUser)) { - return; - } + public function willRenderHovercards(array $objects) { + $viewer = $this->getViewer(); + $phids = mpull($objects, 'getPHID'); - // Reload to get availability. - $user = id(new PhabricatorPeopleQuery()) + $users = id(new PhabricatorPeopleQuery()) ->setViewer($viewer) - ->withIDs(array($user->getID())) + ->withPHIDs($phids) ->needAvailability(true) ->needProfile(true) ->needBadges(true) - ->executeOne(); + ->execute(); + $users = mpull($users, null, 'getPHID'); + + return array( + 'users' => $users, + ); + } + + public function renderHovercard( + PhabricatorHovercardView $hovercard, + PhabricatorObjectHandle $handle, + $object, + $data) { + $viewer = $this->getViewer(); + + $user = idx($data['users'], $object->getPHID()); + if (!$user) { + return; + } $hovercard->setTitle($user->getUsername()); + $profile = $user->getUserProfile(); $detail = $user->getRealName(); if ($profile->getTitle()) { - $detail .= ' - '.$profile->getTitle().'.'; + $detail .= ' - '.$profile->getTitle(); } $hovercard->setDetail($detail); @@ -70,8 +83,6 @@ final class PhabricatorPeopleHovercardEventListener foreach ($badges as $badge) { $hovercard->addBadge($badge); } - - $event->setValue('hovercard', $hovercard); } private function buildBadges( @@ -101,5 +112,4 @@ final class PhabricatorPeopleHovercardEventListener return $items; } - } diff --git a/src/applications/search/application/PhabricatorSearchApplication.php b/src/applications/search/application/PhabricatorSearchApplication.php index f350438616..5b09e21a9e 100644 --- a/src/applications/search/application/PhabricatorSearchApplication.php +++ b/src/applications/search/application/PhabricatorSearchApplication.php @@ -35,7 +35,7 @@ final class PhabricatorSearchApplication extends PhabricatorApplication { 'select/(?P\w+)/(?:(?P\w+)/)?' => 'PhabricatorSearchSelectController', 'index/(?P[^/]+)/' => 'PhabricatorSearchIndexController', - 'hovercard/(?Pretrieve|test)/' + 'hovercard/' => 'PhabricatorSearchHovercardController', 'edit/(?P[^/]+)/' => 'PhabricatorSearchEditController', 'delete/(?P[^/]+)/(?P[^/]+)/' diff --git a/src/applications/search/controller/PhabricatorSearchHovercardController.php b/src/applications/search/controller/PhabricatorSearchHovercardController.php index 98d9aca68b..3993753cc2 100644 --- a/src/applications/search/controller/PhabricatorSearchHovercardController.php +++ b/src/applications/search/controller/PhabricatorSearchHovercardController.php @@ -15,13 +15,43 @@ final class PhabricatorSearchHovercardController ->setViewer($viewer) ->withPHIDs($phids) ->execute(); + $objects = id(new PhabricatorObjectQuery()) ->setViewer($viewer) ->withPHIDs($phids) ->execute(); + $objects = mpull($objects, null, 'getPHID'); + + $extensions = + PhabricatorHovercardEngineExtension::getAllEnabledExtensions(); + + $extension_maps = array(); + foreach ($extensions as $key => $extension) { + $extension->setViewer($viewer); + + $extension_phids = array(); + foreach ($objects as $phid => $object) { + if ($extension->canRenderObjectHovercard($object)) { + $extension_phids[$phid] = $phid; + } + } + + $extension_maps[$key] = $extension_phids; + } + + $extension_data = array(); + foreach ($extensions as $key => $extension) { + $extension_phids = $extension_maps[$key]; + if (!$extension_phids) { + unset($extensions[$key]); + continue; + } + + $extension_data[$key] = $extension->willRenderHovercards( + array_select_keys($objects, $extension_phids)); + } $cards = array(); - foreach ($phids as $phid) { $handle = $handles[$phid]; $object = $objects[$phid]; @@ -32,43 +62,39 @@ final class PhabricatorSearchHovercardController if ($object) { $hovercard->setObject($object); - } - // Send it to the other side of the world, thanks to PhutilEventEngine - $event = new PhabricatorEvent( - PhabricatorEventType::TYPE_UI_DIDRENDERHOVERCARD, - array( - 'hovercard' => $hovercard, - 'handle' => $handle, - 'object' => $object, - )); - $event->setUser($viewer); - PhutilEventEngine::dispatchEvent($event); + foreach ($extension_maps as $key => $extension_phids) { + if (isset($extension_phids[$phid])) { + $extensions[$key]->renderHovercard( + $hovercard, + $handle, + $object, + $extension_data[$key]); + } + } + } $cards[$phid] = $hovercard; } - // Browser-friendly for non-Ajax requests - if (!$request->isAjax()) { - foreach ($cards as $key => $hovercard) { - $cards[$key] = phutil_tag('div', - array( - 'class' => 'ml', - ), - $hovercard); - } - - return $this->buildApplicationPage( - $cards, - array( - 'device' => false, - )); - } else { + if ($request->isAjax()) { return id(new AphrontAjaxResponse())->setContent( array( 'cards' => $cards, )); } + + foreach ($cards as $key => $hovercard) { + $cards[$key] = phutil_tag('div', + array( + 'class' => 'ml', + ), + $hovercard); + } + + return $this->newPage() + ->appendChild($cards) + ->setShowFooter(false); } } diff --git a/src/applications/search/engineextension/PhabricatorHovercardEngineExtension.php b/src/applications/search/engineextension/PhabricatorHovercardEngineExtension.php new file mode 100644 index 0000000000..79eb9b60a4 --- /dev/null +++ b/src/applications/search/engineextension/PhabricatorHovercardEngineExtension.php @@ -0,0 +1,60 @@ +getPhobjectClassConstant('EXTENSIONKEY'); + } + + final public function setViewer($viewer) { + $this->viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + abstract public function isExtensionEnabled(); + + abstract public function getExtensionName(); + + abstract public function canRenderObjectHovercard($object); + + public function getExtensionOrder() { + return 5000; + } + + public function willRenderHovercards(array $objects) { + return null; + } + + abstract public function renderHovercard( + PhabricatorHovercardView $hovercard, + PhabricatorObjectHandle $handle, + $object, + $data); + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getExtensionKey') + ->setSortMethod('getExtensionOrder') + ->execute(); + } + + final public static function getAllEnabledExtensions() { + $extensions = self::getAllExtensions(); + + foreach ($extensions as $key => $extension) { + if (!$extension->isExtensionEnabled()) { + unset($extensions[$key]); + } + } + + return $extensions; + } + +} diff --git a/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php b/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php new file mode 100644 index 0000000000..fa7ca4d33a --- /dev/null +++ b/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php @@ -0,0 +1,55 @@ +getViewer(); + + $extensions = PhabricatorHovercardEngineExtension::getAllExtensions(); + + $rows = array(); + foreach ($extensions as $extension) { + $rows[] = array( + $extension->getExtensionOrder(), + $extension->getExtensionKey(), + get_class($extension), + $extension->getExtensionName(), + $extension->isExtensionEnabled() + ? pht('Yes') + : pht('No'), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Order'), + pht('Key'), + pht('Class'), + pht('Name'), + pht('Enabled'), + )) + ->setColumnClasses( + array( + null, + null, + null, + 'wide pri', + null, + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('HovercardEngine Extensions')) + ->setTable($table); + } + +} diff --git a/src/infrastructure/events/constant/PhabricatorEventType.php b/src/infrastructure/events/constant/PhabricatorEventType.php index 64c58c11e5..c92503376b 100644 --- a/src/infrastructure/events/constant/PhabricatorEventType.php +++ b/src/infrastructure/events/constant/PhabricatorEventType.php @@ -20,8 +20,6 @@ final class PhabricatorEventType extends PhutilEventType { const TYPE_UI_DIDRENDEROBJECTS = 'ui.didRenderObjects'; const TYPE_UI_WILLRENDERPROPERTIES = 'ui.willRenderProperties'; - const TYPE_UI_DIDRENDERHOVERCARD = 'ui.didRenderHovercard'; - const TYPE_PEOPLE_DIDRENDERMENU = 'people.didRenderMenu'; const TYPE_AUTH_WILLREGISTERUSER = 'auth.willRegisterUser'; const TYPE_AUTH_WILLLOGINUSER = 'auth.willLoginUser'; diff --git a/webroot/rsrc/js/core/Hovercard.js b/webroot/rsrc/js/core/Hovercard.js index 15149eceeb..45bece51b3 100644 --- a/webroot/rsrc/js/core/Hovercard.js +++ b/webroot/rsrc/js/core/Hovercard.js @@ -15,7 +15,7 @@ JX.install('Hovercard', { _activeRoot : null, _visiblePHID : null, - fetchUrl : '/search/hovercard/retrieve/', + fetchUrl : '/search/hovercard/', /** * Hovercard storage. {"PHID-XXXX-YYYY":"<...>", ...}