From b8551bb5f98c0bb87004b411a1c8515aea2fa236 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 12 Apr 2019 16:50:59 -0700 Subject: [PATCH] Reduce drag-and-drop jank on dashboards Summary: Depends on D20414. Ref T13272. Several minor things here: - Currently, you can drag panels underneath the invisible "there are no items in this column" div and the "Create Panel / Add Existing Panel" buttons. This is silly; stop it. - Currently, when viewing a tab panel on a dashboard, you can drag the panels inside it. This is extremely silly. Make "movable" off by default and pass it through the async flow only when we actually need it. - Make the whole "Add Tab..." virtual tab clickable to open the dropdown. This removes the rare exception/todo combo I added earlier. {key F} - Add or remove some icons or something. Test Plan: Moved panels around on dashboards. Tried to drag panels inside tab panels. Added tab. Things were less obviously broken. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13272 Differential Revision: https://secure.phabricator.com/D20415 --- resources/celerity/map.php | 42 +++++++++---------- ...bricatorDashboardPanelRenderController.php | 1 + ...habricatorDashboardPanelViewController.php | 1 + ...abricatorDashboardPanelRenderingEngine.php | 3 +- .../PhabricatorDashboardRenderingEngine.php | 27 +++++++++--- .../PhabricatorDashboardTabsPanelType.php | 7 +--- src/view/phui/PHUIListItemView.php | 20 ++++++--- webroot/rsrc/css/phui/phui-action-list.css | 5 +++ .../behavior-dashboard-async-panel.js | 1 + .../behavior-dashboard-move-panels.js | 2 +- 10 files changed, 70 insertions(+), 39 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index b6aaf80f5a..9f41f0584c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,8 +9,8 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '20f3fea5', - 'core.pkg.js' => '69247edd', + 'core.pkg.css' => '3dc188c0', + 'core.pkg.js' => '9ac8af68', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '67e02996', 'diffusion.pkg.css' => '42c75c37', @@ -134,7 +134,7 @@ return array( 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e', 'rsrc/css/phui/object-item/phui-oi-list-view.css' => 'd7723ecc', 'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46', - 'rsrc/css/phui/phui-action-list.css' => '48a45c51', + 'rsrc/css/phui/phui-action-list.css' => 'e820263c', 'rsrc/css/phui/phui-action-panel.css' => '6c386cbf', 'rsrc/css/phui/phui-badge.css' => '666e25ad', 'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d', @@ -371,8 +371,8 @@ return array( 'rsrc/js/application/conpherence/behavior-toggle-widget.js' => '8f959ad0', 'rsrc/js/application/countdown/timer.js' => '6a162524', 'rsrc/js/application/daemon/behavior-bulk-job-reload.js' => '3829a3cf', - 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => 'a871fe00', - 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '7d33143d', + 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '9c01e364', + 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => 'd0a85a85', @@ -594,8 +594,8 @@ return array( 'javelin-behavior-conpherence-search' => '91befbcc', 'javelin-behavior-countdown-timer' => '6a162524', 'javelin-behavior-dark-console' => 'f39d968b', - 'javelin-behavior-dashboard-async-panel' => 'a871fe00', - 'javelin-behavior-dashboard-move-panels' => '7d33143d', + 'javelin-behavior-dashboard-async-panel' => '9c01e364', + 'javelin-behavior-dashboard-move-panels' => 'a2ab19be', 'javelin-behavior-dashboard-query-panel-select' => '1e413dc9', 'javelin-behavior-dashboard-tab-panel' => '0116d3e8', 'javelin-behavior-day-view' => '727a5a61', @@ -757,7 +757,7 @@ return array( 'path-typeahead' => 'ad486db3', 'people-picture-menu-item-css' => 'fe8e07cf', 'people-profile-css' => '2ea2daa1', - 'phabricator-action-list-view-css' => '48a45c51', + 'phabricator-action-list-view-css' => 'e820263c', 'phabricator-busy' => '5202e831', 'phabricator-chatlog-css' => 'abdc76ee', 'phabricator-content-source-view-css' => 'cdf0d579', @@ -1581,14 +1581,6 @@ return array( 'javelin-install', 'javelin-dom', ), - '7d33143d' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phabricator-draggable-list', - ), '80bff3af' => array( 'javelin-install', 'javelin-typeahead-source', @@ -1736,6 +1728,11 @@ return array( 'javelin-install', 'javelin-util', ), + '9c01e364' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-workflow', + ), '9cec214e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1760,6 +1757,14 @@ return array( 'a241536a' => array( 'javelin-install', ), + 'a2ab19be' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phabricator-draggable-list', + ), 'a4356cde' => array( 'javelin-install', 'javelin-dom', @@ -1789,11 +1794,6 @@ return array( 'javelin-install', 'javelin-dom', ), - 'a871fe00' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-workflow', - ), 'a9942052' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelRenderController.php b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelRenderController.php index ebdd4bbb13..fee7ef6de4 100644 --- a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelRenderController.php +++ b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelRenderController.php @@ -36,6 +36,7 @@ final class PhabricatorDashboardPanelRenderController ->setPanel($panel) ->setPanelPHID($panel->getPHID()) ->setParentPanelPHIDs($parent_phids) + ->setMovable($request->getBool('movable')) ->setHeaderMode($request->getStr('headerMode')) ->setPanelKey($request->getStr('panelKey')); diff --git a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php index e06d5716f4..56f9821fcd 100644 --- a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php +++ b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php @@ -40,6 +40,7 @@ final class PhabricatorDashboardPanelViewController $timeline = $this->buildTransactionTimeline( $panel, new PhabricatorDashboardPanelTransactionQuery()); + $timeline->setShouldTerminate(true); $rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine()) ->setViewer($viewer) diff --git a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php index 6a31dc458b..1866815610 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php @@ -12,7 +12,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { private $enableAsyncRendering; private $parentPanelPHIDs; private $headerMode = self::HEADER_MODE_NORMAL; - private $movable = true; + private $movable; private $panelHandle; private $editMode; private $contextObject; @@ -195,6 +195,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { 'headerMode' => $this->getHeaderMode(), 'contextPHID' => $context_phid, 'panelKey' => $this->getPanelKey(), + 'movable' => $this->getMovable(), 'uri' => '/dashboard/panel/render/'.$panel->getID().'/', )); diff --git a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php index 0121dbe3a3..9b63fcac55 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php @@ -80,6 +80,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { ->setParentPanelPHIDs(array()) ->setHeaderMode($h_mode) ->setEditMode($is_editable) + ->setMovable(true) ->setPanelHandle($handles[$panel_phid]); $panel = idx($panels, $panel_phid); @@ -92,9 +93,10 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { $column_classes = $column->getClasses(); + $column_tail = array(); if ($is_editable) { - $column_views[] = $this->renderAddPanelPlaceHolder(); - $column_views[] = $this->renderAddPanelUI($column); + $column_tail[] = $this->renderAddPanelPlaceHolder(); + $column_tail[] = $this->renderAddPanelUI($column); } $sigil = 'dashboard-column'; @@ -103,11 +105,20 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { 'columnKey' => $column->getColumnKey(), ); + $column_view = javelin_tag( + 'div', + array( + 'sigil' => $sigil, + 'meta' => $metadata, + ), + $column_views); + $result->addColumn( - $column_views, - implode(' ', $column_classes), - $sigil, - $metadata); + array( + $column_view, + $column_tail, + ), + implode(' ', $column_classes)); } if ($is_editable) { @@ -159,6 +170,8 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { $create_button = id(new PHUIButtonView()) ->setTag('a') ->setHref($create_uri) + ->setIcon('fa-plus') + ->setColor(PHUIButtonView::GREY) ->setWorkflow(true) ->setText(pht('Create Panel')) ->addClass(PHUI::MARGIN_MEDIUM); @@ -166,6 +179,8 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { $add_button = id(new PHUIButtonView()) ->setTag('a') ->setHref($add_uri) + ->setIcon('fa-window-maximize') + ->setColor(PHUIButtonView::GREY) ->setWorkflow(true) ->setText(pht('Add Existing Panel')) ->addClass(PHUI::MARGIN_MEDIUM); diff --git a/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php b/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php index 5d203b292f..682cc11d78 100644 --- a/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php +++ b/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php @@ -200,10 +200,8 @@ final class PhabricatorDashboardTabsPanelType $list->addMenuItem( id(new PHUIListItemView()) ->setHref('#') - ->setDisabled(true) ->setSelected(false) - ->setName(pht("\xC2\xB7 \xC2\xB7 \xC2\xB7")) - ->setActionIcon('fa-caret-down', '#') + ->setName(pht('Add Tab...')) ->setDropdownMenu($actions)); } @@ -232,7 +230,6 @@ final class PhabricatorDashboardTabsPanelType ->setPanel($subpanel) ->setPanelPHID($subpanel->getPHID()) ->setHeaderMode($no_headers) - ->setMovable(false) ->renderPanel(); } else { $panel_content = pht('(Invalid Panel)'); @@ -257,7 +254,7 @@ final class PhabricatorDashboardTabsPanelType if (!$content) { if ($is_edit) { $message = pht( - 'This tab panel does not have any tabs yet. Use "Add Tab" to '. + 'This tab panel does not have any tabs yet. Use "Add Tab..." to '. 'create or place a tab.'); } else { $message = pht( diff --git a/src/view/phui/PHUIListItemView.php b/src/view/phui/PHUIListItemView.php index 8160919673..5de58b5492 100644 --- a/src/view/phui/PHUIListItemView.php +++ b/src/view/phui/PHUIListItemView.php @@ -245,19 +245,22 @@ final class PHUIListItemView extends AphrontTagView { $classes[] = 'phui-list-item-has-action-icon'; } + $sigil = null; + $metadata = null; if ($this->dropdownMenu) { $classes[] = 'dropdown'; if (!$this->actionIcon) { - throw new Exception( - pht( - 'List item views can not currently render a dropdown without '. - 'an action icon, because no application uses one. Clean up '. - 'PHUICrumbsView, then add this capability.')); + $classes[] = 'dropdown-with-caret'; + Javelin::initBehavior('phui-dropdown-menu'); + $sigil = 'phui-dropdown-menu'; + $metadata = $this->dropdownMenu->getDropdownMenuMetadata(); } } return array( 'class' => $classes, + 'sigil' => $sigil, + 'meta' => $metadata, ); } @@ -372,6 +375,12 @@ final class PHUIListItemView extends AphrontTagView { $this->count); } + $caret = null; + if ($this->dropdownMenu && !$this->actionIcon) { + $caret = id(new PHUIIconView()) + ->setIcon('fa-caret-down'); + } + $icons = $this->getIcons(); $list_item = javelin_tag( @@ -391,6 +400,7 @@ final class PHUIListItemView extends AphrontTagView { $this->renderChildren(), $name, $count, + $caret, )); return array($list_item, $action_link); diff --git a/webroot/rsrc/css/phui/phui-action-list.css b/webroot/rsrc/css/phui/phui-action-list.css index c00e4c08df..0163437b60 100644 --- a/webroot/rsrc/css/phui/phui-action-list.css +++ b/webroot/rsrc/css/phui/phui-action-list.css @@ -237,3 +237,8 @@ background-color: rgba({$alphablue}, 0.07); color: {$sky}; } + +.phui-list-navbar .dropdown-with-caret .phui-list-item-href + .phui-icon-view { + margin-left: 12px; +} diff --git a/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js b/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js index 99336f73c8..126f2d868e 100644 --- a/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js +++ b/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js @@ -13,6 +13,7 @@ JX.behavior('dashboard-async-panel', function(config) { parentPanelPHIDs: config.parentPanelPHIDs.join(','), headerMode: config.headerMode, contextPHID: config.contextPHID, + movable: config.movable, panelKey: config.panelKey }; diff --git a/webroot/rsrc/js/application/dashboard/behavior-dashboard-move-panels.js b/webroot/rsrc/js/application/dashboard/behavior-dashboard-move-panels.js index 8778102b12..c01ab6cd04 100644 --- a/webroot/rsrc/js/application/dashboard/behavior-dashboard-move-panels.js +++ b/webroot/rsrc/js/application/dashboard/behavior-dashboard-move-panels.js @@ -17,7 +17,7 @@ JX.behavior('dashboard-move-panels', function(config) { } function markcolempty(col, toggle) { - JX.DOM.alterClass(col, 'dashboard-column-empty', toggle); + JX.DOM.alterClass(col.parentNode, 'dashboard-column-empty', toggle); } function onupdate(col) {