From 2aac3156f791f05a0c82653aac75379b113e48ad Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 13 Feb 2021 11:23:54 -0800 Subject: [PATCH] Restructure Hovercards to support more context information Summary: Ref T13602. Currently, Hovercards are functions only of the object they represent (and the viewer, etc). Recent changes to how users who can't see an object are rendered motivate making them a function of both the object they represent //and// the context in which they are being viewed. In particular, this enables a hovecard for a user to explain "This user can't see the thing you're lookign at right now.", so visual "exiled" markers can have a path forward toward discovery. Test Plan: - This change isn't expected to affect any behavior. - Viewed hovercards, moused over/out, resized windows, viewed standalone cards, viewed debug cards, saw no behavioral changes. Maniphest Tasks: T13602 Differential Revision: https://secure.phabricator.com/D21553 --- resources/celerity/map.php | 50 ++-- resources/celerity/packages.php | 1 + src/aphront/AphrontRequest.php | 37 +++ .../PhabricatorSearchHovercardController.php | 40 +++- .../PhabricatorSystemDebugUIEventListener.php | 2 +- webroot/rsrc/js/core/Hovercard.js | 167 +------------ webroot/rsrc/js/core/HovercardList.js | 226 ++++++++++++++++++ webroot/rsrc/js/core/behavior-hovercard.js | 68 ++---- 8 files changed, 349 insertions(+), 242 deletions(-) create mode 100644 webroot/rsrc/js/core/HovercardList.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 155ace9d18..06469b117a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => '0e3cf785', 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => '970b3ceb', - 'core.pkg.js' => 'adc34883', + 'core.pkg.js' => '2fe70e3d', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => '5c459f92', 'differential.pkg.js' => '5080baf4', @@ -460,7 +460,8 @@ return array( 'rsrc/js/core/DraggableList.js' => '0169e425', 'rsrc/js/core/Favicon.js' => '7930776a', 'rsrc/js/core/FileUpload.js' => 'ab85e184', - 'rsrc/js/core/Hovercard.js' => '074f0783', + 'rsrc/js/core/Hovercard.js' => 'd9d29a5f', + 'rsrc/js/core/HovercardList.js' => '10a5f4bf', 'rsrc/js/core/KeyboardShortcut.js' => '1a844c06', 'rsrc/js/core/KeyboardShortcutManager.js' => '81debc48', 'rsrc/js/core/MultirowRowManager.js' => '5b54c823', @@ -485,7 +486,7 @@ return array( 'rsrc/js/core/behavior-global-drag-and-drop.js' => '1cab0e9a', 'rsrc/js/core/behavior-high-security-warning.js' => 'dae2d55b', 'rsrc/js/core/behavior-history-install.js' => '6a1583a8', - 'rsrc/js/core/behavior-hovercard.js' => '6c379000', + 'rsrc/js/core/behavior-hovercard.js' => '3f446c72', 'rsrc/js/core/behavior-keyboard-pager.js' => '1325b731', 'rsrc/js/core/behavior-keyboard-shortcuts.js' => '42c44e8b', 'rsrc/js/core/behavior-lightbox-attachments.js' => 'c7e748bf', @@ -670,7 +671,7 @@ return array( 'javelin-behavior-pholio-mock-view' => '5aa1544e', 'javelin-behavior-phui-dropdown-menu' => '5cf0501a', 'javelin-behavior-phui-file-upload' => 'e150bd50', - 'javelin-behavior-phui-hovercards' => '6c379000', + 'javelin-behavior-phui-hovercards' => '3f446c72', 'javelin-behavior-phui-selectable-list' => 'b26a41e4', 'javelin-behavior-phui-submenu' => 'b5e9bff9', 'javelin-behavior-phui-tab-group' => '242aa08b', @@ -858,7 +859,8 @@ return array( 'phui-formation-view-css' => 'd2dec8ed', 'phui-head-thing-view-css' => 'd7f293df', 'phui-header-view-css' => '36c86a58', - 'phui-hovercard' => '074f0783', + 'phui-hovercard' => 'd9d29a5f', + 'phui-hovercard-list' => '10a5f4bf', 'phui-hovercard-view-css' => '6ca90fa0', 'phui-icon-set-selector-css' => '7aa5f3ec', 'phui-icon-view-css' => '4cbc684a', @@ -986,13 +988,6 @@ return array( 'javelin-uri', 'phabricator-notification', ), - '074f0783' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-vector', - 'javelin-request', - 'javelin-uri', - ), '0889b835' => array( 'javelin-install', 'javelin-event', @@ -1030,6 +1025,14 @@ return array( 'javelin-workflow', 'phuix-icon-view', ), + '10a5f4bf' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-vector', + 'javelin-request', + 'javelin-uri', + 'phui-hovercard', + ), '111bfd2d' => array( 'javelin-install', ), @@ -1266,6 +1269,14 @@ return array( 'phabricator-drag-and-drop-file-upload', 'phabricator-draggable-list', ), + '3f446c72' => array( + 'javelin-behavior', + 'javelin-behavior-device', + 'javelin-stratcom', + 'javelin-vector', + 'phui-hovercard', + 'phui-hovercard-list', + ), '407ee861' => array( 'javelin-behavior', 'javelin-uri', @@ -1557,13 +1568,6 @@ return array( 'javelin-workflow', 'javelin-magical-init', ), - '6c379000' => array( - 'javelin-behavior', - 'javelin-behavior-device', - 'javelin-stratcom', - 'javelin-vector', - 'phui-hovercard', - ), '6cfa0008' => array( 'javelin-dom', 'javelin-dynval', @@ -2132,6 +2136,13 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), + 'd9d29a5f' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-vector', + 'javelin-request', + 'javelin-uri', + ), 'da15d3dc' => array( 'phui-oi-list-view-css', ), @@ -2367,6 +2378,7 @@ return array( 'javelin-behavior-global-drag-and-drop', 'javelin-behavior-phabricator-reveal-content', 'phui-hovercard', + 'phui-hovercard-list', 'javelin-behavior-phui-hovercards', 'javelin-color', 'javelin-fx', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index a5dabb6014..47e13932de 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -60,6 +60,7 @@ return array( 'javelin-behavior-global-drag-and-drop', 'javelin-behavior-phabricator-reveal-content', 'phui-hovercard', + 'phui-hovercard-list', 'javelin-behavior-phui-hovercards', 'javelin-color', 'javelin-fx', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index d8310386c2..a3fa05cca2 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -224,6 +224,43 @@ final class AphrontRequest extends Phobject { } + /** + * @task data + */ + public function getJSONMap($name, $default = array()) { + if (!isset($this->requestData[$name])) { + return $default; + } + + $raw_data = phutil_string_cast($this->requestData[$name]); + $raw_data = trim($raw_data); + if (!strlen($raw_data)) { + return $default; + } + + if ($raw_data[0] !== '{') { + throw new Exception( + pht( + 'Request parameter "%s" is not formatted properly. Expected a '. + 'JSON object, but value does not start with "{".', + $name)); + } + + try { + $json_object = phutil_json_decode($raw_data); + } catch (PhutilJSONParserException $ex) { + throw new Exception( + pht( + 'Request parameter "%s" is not formatted properly. Expected a '. + 'JSON object, but encountered a syntax error: %s.', + $name, + $ex->getMessage())); + } + + return $json_object; + } + + /** * @task data */ diff --git a/src/applications/search/controller/PhabricatorSearchHovercardController.php b/src/applications/search/controller/PhabricatorSearchHovercardController.php index ca83d1896d..0eaae1a138 100644 --- a/src/applications/search/controller/PhabricatorSearchHovercardController.php +++ b/src/applications/search/controller/PhabricatorSearchHovercardController.php @@ -9,7 +9,8 @@ final class PhabricatorSearchHovercardController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $phids = $request->getArr('phids'); + + $cards = $request->getJSONMap('cards'); // If object names are provided, look them up and pretend they were // passed as additional PHIDs. This is primarily useful for debugging, @@ -23,18 +24,29 @@ final class PhabricatorSearchHovercardController ->execute(); foreach ($named_objects as $object) { - $phids[] = $object->getPHID(); + $cards[] = array( + 'objectPHID' => $object->getPHID(), + ); } } + $object_phids = array(); + $handle_phids = array(); + foreach ($cards as $card) { + $object_phid = idx($card, 'objectPHID'); + + $handle_phids[] = $object_phid; + $object_phids[] = $object_phid; + } + $handles = id(new PhabricatorHandleQuery()) ->setViewer($viewer) - ->withPHIDs($phids) + ->withPHIDs($handle_phids) ->execute(); $objects = id(new PhabricatorObjectQuery()) ->setViewer($viewer) - ->withPHIDs($phids) + ->withPHIDs($object_phids) ->execute(); $objects = mpull($objects, null, 'getPHID'); @@ -67,10 +79,12 @@ final class PhabricatorSearchHovercardController array_select_keys($objects, $extension_phids)); } - $cards = array(); - foreach ($phids as $phid) { - $handle = $handles[$phid]; - $object = idx($objects, $phid); + $results = array(); + foreach ($cards as $card_key => $card) { + $object_phid = $card['objectPHID']; + + $handle = $handles[$object_phid]; + $object = idx($objects, $object_phid); $hovercard = id(new PHUIHovercardView()) ->setUser($viewer) @@ -90,18 +104,18 @@ final class PhabricatorSearchHovercardController } } - $cards[$phid] = $hovercard; + $results[$card_key] = $hovercard; } if ($request->isAjax()) { return id(new AphrontAjaxResponse())->setContent( array( - 'cards' => $cards, + 'cards' => $results, )); } - foreach ($cards as $key => $hovercard) { - $cards[$key] = phutil_tag('div', + foreach ($results as $key => $hovercard) { + $results[$key] = phutil_tag('div', array( 'class' => 'ml', ), @@ -109,7 +123,7 @@ final class PhabricatorSearchHovercardController } return $this->newPage() - ->appendChild($cards) + ->appendChild($results) ->setShowFooter(false); } diff --git a/src/applications/system/events/PhabricatorSystemDebugUIEventListener.php b/src/applications/system/events/PhabricatorSystemDebugUIEventListener.php index 18b94323b6..5bfb6843c4 100644 --- a/src/applications/system/events/PhabricatorSystemDebugUIEventListener.php +++ b/src/applications/system/events/PhabricatorSystemDebugUIEventListener.php @@ -42,7 +42,7 @@ final class PhabricatorSystemDebugUIEventListener $submenu[] = id(new PhabricatorActionView()) ->setIcon('fa-address-card-o') ->setName(pht('View Hovercard')) - ->setHref(urisprintf('/search/hovercard/?phids[]=%s', $phid)); + ->setHref(urisprintf('/search/hovercard/?names=%s', $phid)); $developer_action = id(new PhabricatorActionView()) ->setName(pht('Advanced/Developer...')) diff --git a/webroot/rsrc/js/core/Hovercard.js b/webroot/rsrc/js/core/Hovercard.js index 65e1455252..63fc8eab7f 100644 --- a/webroot/rsrc/js/core/Hovercard.js +++ b/webroot/rsrc/js/core/Hovercard.js @@ -10,163 +10,18 @@ JX.install('Hovercard', { - statics : { - _node : null, - _activeRoot : null, - _visiblePHID : null, - _alignment: null, + properties: { + hovercardKey: null, + objectPHID: null, + isLoading: false, + isLoaded: false, + content: null + }, - fetchUrl : '/search/hovercard/', - - /** - * Hovercard storage. {"PHID-XXXX-YYYY":"<...>", ...} - */ - _cards : {}, - - getAnchor : function() { - return this._activeRoot; - }, - - getCard : function() { - var self = JX.Hovercard; - return self._node; - }, - - getAlignment: function() { - var self = JX.Hovercard; - return self._alignment; - }, - - show : function(root, phid) { - var self = JX.Hovercard; - - if (root === this._activeRoot) { - return; - } - - self.hide(); - - self._visiblePHID = phid; - self._activeRoot = root; - - if (!(phid in self._cards)) { - self._load([phid]); - } else { - self._drawCard(phid); - } - }, - - _drawCard : function(phid) { - var self = JX.Hovercard; - // card is loading... - if (self._cards[phid] === true) { - return; - } - // Not the current requested card - if (phid != self._visiblePHID) { - return; - } - // Not loaded - if (!(phid in self._cards)) { - return; - } - - var root = self._activeRoot; - var node = JX.$N('div', - { className: 'jx-hovercard-container' }, - JX.$H(self._cards[phid])); - - self._node = node; - - // Append the card to the document, but offscreen, so we can measure it. - node.style.left = '-10000px'; - document.body.appendChild(node); - - // Retrieve size from child (wrapper), since node gives wrong dimensions? - var child = node.firstChild; - var p = JX.$V(root); - var d = JX.Vector.getDim(root); - var n = JX.Vector.getDim(child); - var v = JX.Vector.getViewport(); - var s = JX.Vector.getScroll(); - - // Move the tip so it's nicely aligned. - var margin = 20; - - - // Try to align the card directly above the link, with left borders - // touching. - var x = p.x; - - // If this would push us off the right side of the viewport, push things - // back to the left. - if ((x + n.x + margin) > (s.x + v.x)) { - x = (s.x + v.x) - n.x - margin; - } - - // Try to put the card above the link. - var y = p.y - n.y - margin; - self._alignment = 'north'; - - // If the card is near the top of the window, show it beneath the - // link we're hovering over instead. - if ((y - margin) < s.y) { - y = p.y + d.y + margin; - self._alignment = 'south'; - } - - node.style.left = x + 'px'; - node.style.top = y + 'px'; - }, - - hide : function() { - var self = JX.Hovercard; - self._visiblePHID = null; - self._activeRoot = null; - if (self._node) { - JX.DOM.remove(self._node); - self._node = null; - } - }, - - /** - * Pass it an array of phids to load them into storage - * - * @param list phids - */ - _load : function(phids) { - var self = JX.Hovercard; - var uri = JX.$U(self.fetchUrl); - - var send = false; - for (var ii = 0; ii < phids.length; ii++) { - var phid = phids[ii]; - if (phid in self._cards) { - continue; - } - self._cards[phid] = true; // means "loading" - uri.setQueryParam('phids['+ii+']', phids[ii]); - send = true; - } - - if (!send) { - // already loaded / loading everything! - return; - } - - new JX.Request(uri, function(r) { - for (var phid in r.cards) { - self._cards[phid] = r.cards[phid]; - - // Don't draw if the user is faster than the browser - // Only draw if the user is still requesting the original card - if (self.getCard() && phid != self._visiblePHID) { - continue; - } - - self._drawCard(phid); - } - }).send(); + members: { + newContentNode: function() { + return JX.$H(this.getContent()); } } + }); diff --git a/webroot/rsrc/js/core/HovercardList.js b/webroot/rsrc/js/core/HovercardList.js new file mode 100644 index 0000000000..91a575d68d --- /dev/null +++ b/webroot/rsrc/js/core/HovercardList.js @@ -0,0 +1,226 @@ +/** + * @requires javelin-install + * javelin-dom + * javelin-vector + * javelin-request + * javelin-uri + * phui-hovercard + * @provides phui-hovercard-list + * @javelin + */ + +JX.install('HovercardList', { + + construct: function() { + this._cards = {}; + this._drawRequest = {}; + }, + + members: { + _cardNode: null, + _rootNode: null, + _cards: null, + _drawRequest: null, + _visibleCard: null, + + _fetchURI : '/search/hovercard/', + + getCard: function(spec) { + var hovercard_key = this._newHovercardKey(spec); + + if (!(hovercard_key in this._cards)) { + var card = new JX.Hovercard() + .setHovercardKey(hovercard_key) + .setObjectPHID(spec.hoverPHID); + + this._cards[hovercard_key] = card; + } + + return this._cards[hovercard_key]; + }, + + drawCard: function(card, node) { + this._drawRequest = { + card: card, + node: node + }; + + if (card.getIsLoaded()) { + return this._paintCard(card); + } + + if (card.getIsLoading()) { + return; + } + + var hovercard_key = card.getHovercardKey(); + + var request = {}; + request[hovercard_key] = this._newCardRequest(card); + request = JX.JSON.stringify(request); + + var uri = JX.$U(this._fetchURI) + .setQueryParam('cards', request); + + var onresponse = JX.bind(this, function(r) { + var card = this._cards[hovercard_key]; + + this._fillCard(card, r.cards[hovercard_key]); + this._paintCard(card); + }); + + card.setIsLoading(true); + + new JX.Request(uri, onresponse) + .send(); + }, + + _newHovercardKey: function(spec) { + return 'phid=' + spec.hoverPHID; + }, + + _newCardRequest: function(card) { + return { + objectPHID: card.getObjectPHID() + }; + }, + + _getCardNode: function() { + if (!this._cardNode) { + var attributes = { + className: 'jx-hovercard-container' + }; + + this._cardNode = JX.$N('div', attributes); + } + + return this._cardNode; + }, + + _fillCard: function(card, response) { + card.setContent(response); + card.setIsLoaded(true); + }, + + _paintCard: function(card) { + var request = this._drawRequest; + + if (request.card !== card) { + // This paint request is no longer the most recent paint request. + return; + } + + this.hideCard(); + + this._rootNode = request.node; + var root = this._rootNode; + var node = this._getCardNode(); + + JX.DOM.setContent(node, card.newContentNode()); + + // Append the card to the document, but offscreen, so we can measure it. + node.style.left = '-10000px'; + document.body.appendChild(node); + + // Retrieve size from child (wrapper), since node gives wrong dimensions? + var child = node.firstChild; + + var p = JX.$V(root); + var d = JX.Vector.getDim(root); + var n = JX.Vector.getDim(child); + var v = JX.Vector.getViewport(); + var s = JX.Vector.getScroll(); + + // Move the tip so it's nicely aligned. + var margin = 20; + + // Try to align the card directly above the link, with left borders + // touching. + var x = p.x; + + // If this would push us off the right side of the viewport, push things + // back to the left. + if ((x + n.x + margin) > (s.x + v.x)) { + x = (s.x + v.x) - n.x - margin; + } + + // Try to put the card above the link. + var y = p.y - n.y - margin; + + var alignment = 'north'; + + // If the card is near the top of the window, show it beneath the + // link we're hovering over instead. + if ((y - margin) < s.y) { + y = p.y + d.y + margin; + alignment = 'south'; + } + + this._alignment = alignment; + node.style.left = x + 'px'; + node.style.top = y + 'px'; + + this._visibleCard = card; + }, + + hideCard: function() { + var node = this._getCardNode(); + JX.DOM.remove(node); + + this._rootNode = null; + this._alignment = null; + this._visibleCard = null; + }, + + onMouseMove: function(e) { + if (!this._visibleCard) { + return; + } + + var root = this._rootNode; + var node = this._getCardNode(); + var alignment = this._alignment; + + var mouse = JX.$V(e); + var node_pos = JX.$V(node); + var node_dim = JX.Vector.getDim(node); + var root_pos = JX.$V(root); + var root_dim = JX.Vector.getDim(root); + + var margin = 20; + + if (alignment === 'south') { + // Cursor is below the node. + if (mouse.y > node_pos.y + node_dim.y + margin) { + this.hideCard(); + } + + // Cursor is above the root. + if (mouse.y < root_pos.y - margin) { + this.hideCard(); + } + } else { + // Cursor is above the node. + if (mouse.y < node_pos.y - margin) { + this.hideCard(); + } + + // Cursor is below the root. + if (mouse.y > root_pos.y + root_dim.y + margin) { + this.hideCard(); + } + } + + // Cursor is too far to the left. + if (mouse.x < Math.min(root_pos.x, node_pos.x) - margin) { + this.hideCard(); + } + + // Cursor is too far to the right. + if (mouse.x > + Math.max(root_pos.x + root_dim.x, node_pos.x + node_dim.x) + margin) { + this.hideCard(); + } + } + } +}); diff --git a/webroot/rsrc/js/core/behavior-hovercard.js b/webroot/rsrc/js/core/behavior-hovercard.js index 8a1c610752..d5caccf650 100644 --- a/webroot/rsrc/js/core/behavior-hovercard.js +++ b/webroot/rsrc/js/core/behavior-hovercard.js @@ -5,10 +5,18 @@ * javelin-stratcom * javelin-vector * phui-hovercard + * phui-hovercard-list * @javelin */ -JX.behavior('phui-hovercards', function() { +JX.behavior('phui-hovercards', function(config, statics) { + if (statics.hovercardList) { + return; + } + + var cards = new JX.HovercardList(); + statics.hovercardList = cards; + // We listen for mousemove instead of mouseover to handle the case when user // scrolls with keyboard. We don't want to display hovercard if node gets @@ -23,65 +31,19 @@ JX.behavior('phui-hovercards', function() { return; } + var node = e.getNode('hovercard'); var data = e.getNodeData('hovercard'); - JX.Hovercard.show( - e.getNode('hovercard'), - data.hoverPHID); + var card = cards.getCard(data); + + cards.drawCard(card, node); }); JX.Stratcom.listen( 'mousemove', null, function (e) { - if (!JX.Hovercard.getCard()) { - return; - } - - var root = JX.Hovercard.getAnchor(); - var node = JX.Hovercard.getCard(); - var align = JX.Hovercard.getAlignment(); - - var mouse = JX.$V(e); - var node_pos = JX.$V(node); - var node_dim = JX.Vector.getDim(node); - var root_pos = JX.$V(root); - var root_dim = JX.Vector.getDim(root); - - var margin = 20; - - if (align == 'south') { - // Cursor is below the node. - if (mouse.y > node_pos.y + node_dim.y + margin) { - JX.Hovercard.hide(); - } - - // Cursor is above the root. - if (mouse.y < root_pos.y - margin) { - JX.Hovercard.hide(); - } - } else { - // Cursor is above the node. - if (mouse.y < node_pos.y - margin) { - JX.Hovercard.hide(); - } - - // Cursor is below the root. - if (mouse.y > root_pos.y + root_dim.y + margin) { - JX.Hovercard.hide(); - } - } - - // Cursor is too far to the left. - if (mouse.x < Math.min(root_pos.x, node_pos.x) - margin) { - JX.Hovercard.hide(); - } - - // Cursor is too far to the right. - if (mouse.x > - Math.max(root_pos.x + root_dim.x, node_pos.x + node_dim.x) + margin) { - JX.Hovercard.hide(); - } + cards.onMouseMove(e); }); // When we leave the page, hide any visible hovercards. If we don't do this, @@ -91,7 +53,7 @@ JX.behavior('phui-hovercards', function() { ['unload', 'onresize'], null, function() { - JX.Hovercard.hide(); + cards.hideCard(); }); });