From 9ad9ac9be69530c2c186d408e9a7048bf242a2e3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 11 Apr 2019 07:12:13 -0700 Subject: [PATCH] On Dashboard tab panels in edit mode, make the "Tab Name" and the "Dropdown Edit Caret" into different links Summary: Ref T13272. In edit mode, tab panels now have a dropdown menu. However, this sort of overrlaps with the actual action of clicking the tab to select it. Separate these into different click targets so that "select tab X" and "open dropdown menu for X" are different operations. This is more work than it appears because: - We have an "action icon" already, used when you put a dashboard on a portal/home to create an "Edit" link. It makes sense to attach dropdowns to this, but it has some hard-coded stuff. - In applications with a "Create " in the crumbs (like Maniphest), we may use a dropdown menu if there are multiple create forms available. However, this menu renders in a weird way by reading all the properties out of an actual "View" object and building something else. - The "list of tabs" stuff shares code with different "list of tabs" navigation used by Diffusion and Instances. ..but I think I fixed everything and didn't break anything. Test Plan: - Clicked "select tab" and "open dropdown menu" as separate actions. - Viewed Diffusion, Maniphest with multiple create forms, Instances. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13272 Differential Revision: https://secure.phabricator.com/D20396 --- resources/celerity/map.php | 26 +++--- ...habricatorDashboardPanelViewController.php | 59 ++----------- ...abricatorDashboardPanelRenderingEngine.php | 8 +- .../PhabricatorDashboardRenderingEngine.php | 1 + .../PhabricatorDashboardTabsPanelType.php | 8 +- src/view/phui/PHUICrumbsView.php | 8 +- src/view/phui/PHUIListItemView.php | 82 +++++++++++++------ webroot/rsrc/css/phui/phui-action-list.css | 27 ++++-- webroot/rsrc/css/phui/phui-list.css | 21 +++-- .../dashboard/behavior-dashboard-tab-panel.js | 8 ++ 10 files changed, 135 insertions(+), 113 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 39a0d4cf61..e33fdcbacf 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' => 'dacb981b', - 'core.pkg.js' => 'c783d8f6', + 'core.pkg.css' => '9d654dff', + 'core.pkg.js' => '350acda5', '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' => 'f14f2422', 'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46', - 'rsrc/css/phui/phui-action-list.css' => '8862282e', + 'rsrc/css/phui/phui-action-list.css' => '48a45c51', 'rsrc/css/phui/phui-action-panel.css' => '6c386cbf', 'rsrc/css/phui/phui-badge.css' => '666e25ad', 'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d', @@ -164,7 +164,7 @@ return array( 'rsrc/css/phui/phui-invisible-character-view.css' => 'c694c4a4', 'rsrc/css/phui/phui-left-right.css' => '68513c34', 'rsrc/css/phui/phui-lightbox.css' => '4ebf22da', - 'rsrc/css/phui/phui-list.css' => '470b1adb', + 'rsrc/css/phui/phui-list.css' => '734a1039', 'rsrc/css/phui/phui-object-box.css' => 'f434b6be', 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', @@ -374,7 +374,7 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '09ecf50c', 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '076bd092', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', - 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '9b1cbd76', + 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '8d4490a2', 'rsrc/js/application/diff/DiffChangeset.js' => 'd0a85a85', 'rsrc/js/application/diff/DiffChangesetList.js' => '04023d82', 'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94', @@ -597,7 +597,7 @@ return array( 'javelin-behavior-dashboard-async-panel' => '09ecf50c', 'javelin-behavior-dashboard-move-panels' => '076bd092', 'javelin-behavior-dashboard-query-panel-select' => '1e413dc9', - 'javelin-behavior-dashboard-tab-panel' => '9b1cbd76', + 'javelin-behavior-dashboard-tab-panel' => '8d4490a2', 'javelin-behavior-day-view' => '727a5a61', 'javelin-behavior-desktop-notifications-control' => '070679fe', 'javelin-behavior-detect-timezone' => '78bc5d94', @@ -757,7 +757,7 @@ return array( 'path-typeahead' => 'ad486db3', 'people-picture-menu-item-css' => 'fe8e07cf', 'people-profile-css' => '2ea2daa1', - 'phabricator-action-list-view-css' => '8862282e', + 'phabricator-action-list-view-css' => '48a45c51', 'phabricator-busy' => '5202e831', 'phabricator-chatlog-css' => 'abdc76ee', 'phabricator-content-source-view-css' => 'cdf0d579', @@ -847,7 +847,7 @@ return array( 'phui-invisible-character-view-css' => 'c694c4a4', 'phui-left-right-css' => '68513c34', 'phui-lightbox-css' => '4ebf22da', - 'phui-list-view-css' => '470b1adb', + 'phui-list-view-css' => '734a1039', 'phui-object-box-css' => 'f434b6be', 'phui-oi-big-ui-css' => 'fa74cc35', 'phui-oi-color-css' => 'b517bfa0', @@ -1644,6 +1644,11 @@ return array( 'phabricator-shaped-request', 'conpherence-thread-manager', ), + '8d4490a2' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + ), '8e0aa661' => array( 'javelin-install', 'javelin-dom', @@ -1736,11 +1741,6 @@ return array( 'javelin-install', 'javelin-util', ), - '9b1cbd76' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - ), '9cec214e' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php index 4b5f1b45be..3c56ec4674 100644 --- a/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php +++ b/src/applications/dashboard/controller/panel/PhabricatorDashboardPanelViewController.php @@ -19,6 +19,11 @@ final class PhabricatorDashboardPanelViewController return new Aphront404Response(); } + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $panel, + PhabricatorPolicyCapability::CAN_EDIT); + $title = $panel->getMonogram().' '.$panel->getName(); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb( @@ -29,7 +34,6 @@ final class PhabricatorDashboardPanelViewController $header = $this->buildHeaderView($panel); $curtain = $this->buildCurtainView($panel); - $properties = $this->buildPropertyView($panel); $timeline = $this->buildTransactionTimeline( $panel, @@ -40,6 +44,7 @@ final class PhabricatorDashboardPanelViewController ->setPanel($panel) ->setPanelPHID($panel->getPHID()) ->setParentPanelPHIDs(array()) + ->setEditMode(true) ->renderPanel(); $preview = id(new PHUIBoxView()) @@ -50,10 +55,9 @@ final class PhabricatorDashboardPanelViewController ->setHeader($header) ->setCurtain($curtain) ->setMainColumn(array( - $properties, + $rendered_panel, $timeline, - )) - ->setFooter($rendered_panel); + )); return $this->newPage() ->setTitle($title) @@ -124,51 +128,4 @@ final class PhabricatorDashboardPanelViewController return $curtain; } - private function buildPropertyView(PhabricatorDashboardPanel $panel) { - $viewer = $this->getViewer(); - - $properties = id(new PHUIPropertyListView()) - ->setUser($viewer); - - $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $viewer, - $panel); - - $panel_type = $panel->getImplementation(); - if ($panel_type) { - $type_name = $panel_type->getPanelTypeName(); - } else { - $type_name = phutil_tag( - 'em', - array(), - nonempty($panel->getPanelType(), pht('null'))); - } - - $properties->addProperty( - pht('Panel Type'), - $type_name); - - $properties->addProperty( - pht('Editable By'), - $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - - $dashboard_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $panel->getPHID(), - PhabricatorDashboardPanelHasDashboardEdgeType::EDGECONST); - - $does_not_appear = pht( - 'This panel does not appear on any dashboards.'); - - $properties->addProperty( - pht('Appears On'), - $dashboard_phids - ? $viewer->renderHandleList($dashboard_phids) - : phutil_tag('em', array(), $does_not_appear)); - - return id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Details')) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->addPropertyList($properties); - } - } diff --git a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php index a5ae1fcfcb..1320a7c986 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php @@ -15,6 +15,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { private $dashboardID; private $movable = true; private $panelHandle; + private $editMode; public function setDashboardID($id) { $this->dashboardID = $id; @@ -44,7 +45,12 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { } public function isEditMode() { - return ($this->getHeaderMode() === self::HEADER_MODE_EDIT); + return $this->editMode; + } + + public function setEditMode($mode) { + $this->editMode = $mode; + return $this; } /** diff --git a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php index 9ff02a3903..dcb35a2cb6 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php @@ -76,6 +76,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { ->setPanelPHID($panel_phid) ->setParentPanelPHIDs(array()) ->setHeaderMode($h_mode) + ->setEditMode($is_editable) ->setPanelHandle($handles[$panel_phid]); $panel = idx($panels, $panel_phid); diff --git a/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php b/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php index 34873f2a9c..1fae501c19 100644 --- a/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php +++ b/src/applications/dashboard/paneltype/PhabricatorDashboardTabsPanelType.php @@ -166,7 +166,9 @@ final class PhabricatorDashboardTabsPanelType ->setHref($details_uri) ->setDisabled(!$subpanel)); - $tab_view->setDropdownMenu($dropdown_menu); + $tab_view + ->setActionIcon('fa-caret-down', '#') + ->setDropdownMenu($dropdown_menu); } $list->addMenuItem($tab_view); @@ -193,8 +195,10 @@ final class PhabricatorDashboardTabsPanelType $list->addMenuItem( id(new PHUIListItemView()) ->setHref('#') + ->setDisabled(true) ->setSelected(false) - ->setName(pht('Add Tab...')) + ->setName(pht("\xC2\xB7 \xC2\xB7 \xC2\xB7")) + ->setActionIcon('fa-caret-down', '#') ->setDropdownMenu($actions)); } diff --git a/src/view/phui/PHUICrumbsView.php b/src/view/phui/PHUICrumbsView.php index 00a1f1911e..fb1a0ae9b4 100644 --- a/src/view/phui/PHUICrumbsView.php +++ b/src/view/phui/PHUICrumbsView.php @@ -50,9 +50,15 @@ final class PHUICrumbsView extends AphrontView { $action_view = null; if ($this->actions) { + // TODO: This block of code takes "PHUIListItemView" objects and turns + // them into some weird abomination by reading most of their properties + // out. Some day, this workflow should render the items and CSS should + // resytle them in place without needing a wholly separate set of + // DOM nodes. + $actions = array(); foreach ($this->actions as $action) { - if ($action->getType() == PHUIListItemView::TYPE_DIVIDER) { + if ($action->getType() == PHUIListItemView::TYPE_DIVIDER) { $actions[] = phutil_tag( 'span', array( diff --git a/src/view/phui/PHUIListItemView.php b/src/view/phui/PHUIListItemView.php index 79b6c89df1..8160919673 100644 --- a/src/view/phui/PHUIListItemView.php +++ b/src/view/phui/PHUIListItemView.php @@ -35,7 +35,7 @@ final class PHUIListItemView extends AphrontTagView { private $actionIconHref; private $count; private $rel; - private $hasDropdown; + private $dropdownMenu; public function setOpenInNewWindow($open_in_new_window) { $this->openInNewWindow = $open_in_new_window; @@ -65,11 +65,19 @@ final class PHUIListItemView extends AphrontTagView { } public function setDropdownMenu(PhabricatorActionListView $actions) { - Javelin::initBehavior('phui-dropdown-menu'); - $this->addSigil('phui-dropdown-menu'); - $this->setMetadata($actions->getDropdownMenuMetadata()); - $this->hasDropdown = true; + $this->dropdownMenu = $actions; + + // TODO: "PHUICrumbsView" currently creates a bad copy of list items + // by reading some of their properties. To survive this copy step, we + // need to mutate "$this" immediately or the "Create Object" dropdown + // when multiple create forms exist breaks. + + if (!$this->actionIcon) { + Javelin::initBehavior('phui-dropdown-menu'); + $this->addSigil('phui-dropdown-menu'); + $this->setMetadata($actions->getDropdownMenuMetadata()); + } return $this; } @@ -237,12 +245,19 @@ final class PHUIListItemView extends AphrontTagView { $classes[] = 'phui-list-item-has-action-icon'; } - if ($this->hasDropdown) { + 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.')); + } } return array( - 'class' => implode(' ', $classes), + 'class' => $classes, ); } @@ -345,19 +360,7 @@ final class PHUIListItemView extends AphrontTagView { $classes[] = 'phui-list-item-indented'; } - $action_link = null; - if ($this->actionIcon) { - $action_icon = id(new PHUIIconView()) - ->setIcon($this->actionIcon) - ->addClass('phui-list-item-action-icon'); - $action_link = phutil_tag( - 'a', - array( - 'href' => $this->actionIconHref, - 'class' => 'phui-list-item-action-href', - ), - $action_icon); - } + $action_link = $this->newActionIconView(); $count = null; if ($this->count) { @@ -369,12 +372,6 @@ final class PHUIListItemView extends AphrontTagView { $this->count); } - if ($this->hasDropdown) { - $caret = phutil_tag('span', array('class' => 'caret'), ''); - } else { - $caret = null; - } - $icons = $this->getIcons(); $list_item = javelin_tag( @@ -393,11 +390,42 @@ final class PHUIListItemView extends AphrontTagView { $icons, $this->renderChildren(), $name, - $caret, $count, )); return array($list_item, $action_link); } + private function newActionIconView() { + $action_icon = $this->actionIcon; + $action_href = $this->actionIconHref; + + if ($action_icon === null) { + return null; + } + + $icon_view = id(new PHUIIconView()) + ->setIcon($action_icon) + ->addClass('phui-list-item-action-icon'); + + if ($this->dropdownMenu) { + Javelin::initBehavior('phui-dropdown-menu'); + $sigil = 'phui-dropdown-menu'; + $metadata = $this->dropdownMenu->getDropdownMenuMetadata(); + } else { + $sigil = null; + $metadata = null; + } + + return javelin_tag( + 'a', + array( + 'href' => $action_href, + 'class' => 'phui-list-item-action-href', + 'sigil' => $sigil, + 'meta' => $metadata, + ), + $icon_view); + } + } diff --git a/webroot/rsrc/css/phui/phui-action-list.css b/webroot/rsrc/css/phui/phui-action-list.css index 6697427562..c00e4c08df 100644 --- a/webroot/rsrc/css/phui/phui-action-list.css +++ b/webroot/rsrc/css/phui/phui-action-list.css @@ -214,13 +214,26 @@ color: {$sky}; } -.phui-list-navbar .phui-list-item-view.dropdown .phui-list-item-href { - padding-right: 28px; +.phui-list-navbar .phui-list-item-href { + display: inline-block; } -.phui-list-navbar .phui-list-item-view .caret { - position: absolute; - top: 6px; - right: 12px; - border-top: 7px solid {$greytext}; +.phui-list-navbar .phui-list-item-disabled .phui-list-item-href { + color: {$lightgreytext}; +} + +.phui-list-navbar .phui-list-item-action-href { + display: inline-block; + padding: 8px 16px; + line-height: 16px; +} + +.phui-list-navbar .phui-list-item-action-href .phui-icon-view { + color: {$darkgreytext}; +} + +.device-desktop + .phui-list-navbar .phui-list-item-action-href:hover { + background-color: rgba({$alphablue}, 0.07); + color: {$sky}; } diff --git a/webroot/rsrc/css/phui/phui-list.css b/webroot/rsrc/css/phui/phui-list.css index 5fab89bcd8..bb93f70f26 100644 --- a/webroot/rsrc/css/phui/phui-list.css +++ b/webroot/rsrc/css/phui/phui-list.css @@ -110,10 +110,6 @@ border-right: 1px solid {$thinblueborder}; } -.phui-list-view.phui-list-navbar > li > * { - display: block; -} - .phui-list-navbar .phui-list-item-href { color: {$bluetext}; padding: 8px 16px; @@ -265,7 +261,7 @@ /* - Action Icon ----------------------------------------------------------- */ -.phui-list-item-has-action-icon .phui-list-item-action-href { +.phui-list-sidenav .phui-list-item-has-action-icon .phui-list-item-action-href { position: absolute; width: 28px; top: 0; @@ -277,26 +273,29 @@ display: none; } -.phui-list-item-has-action-icon.phui-list-item-selected .phui-list-item-href { +.phui-list-sidenav .phui-list-item-has-action-icon.phui-list-item-selected + .phui-list-item-href { padding-right: 32px; } -.phui-list-item-has-action-icon.phui-list-item-selected +.phui-list-sidenav .phui-list-item-has-action-icon.phui-list-item-selected .phui-list-item-action-href { display: block; } -.phui-list-item-has-action-icon .phui-list-item-action-href:hover { +.phui-list-sidenav .phui-list-item-has-action-icon + .phui-list-item-action-href:hover { background-color: rgba({$alphablack},.05); } -.phui-list-item-has-action-icon .phui-list-item-action-icon { +.phui-list-sidenav .phui-list-item-has-action-icon .phui-list-item-action-icon { opacity: 0.5; } -.phui-list-item-has-action-icon .phui-list-item-action-href:hover +.phui-list-sidenav .phui-list-item-has-action-icon + .phui-list-item-action-href:hover .phui-list-item-action-icon { - opacity: 1; + opacity: 1; } /* - Item Counts ----------------------------------------------------------- */ diff --git a/webroot/rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js b/webroot/rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js index b9c70d7df9..7736cefa69 100644 --- a/webroot/rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js +++ b/webroot/rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js @@ -8,6 +8,14 @@ JX.behavior('dashboard-tab-panel', function() { JX.Stratcom.listen('click', 'dashboard-tab-panel-tab', function(e) { + // On dashboard panels in edit mode, the user may click the dropdown caret + // within a tab to open the context menu. If they do, this click should + // just open the menu, not select the tab. For now, pass the event here + // to let the menu handler act on it. + if (JX.Stratcom.pass(e)) { + return; + } + e.kill(); var ii;