1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 10:12:41 +01:00

Replace the "Choose Subtype" radio buttons dialog with a simpler "big stuff you click" sort of UI

Summary:
Ref T13222. Fixes T12588. See PHI683. In several cases, we present the user with a choice between multiple major options: Alamnac service types, Drydock blueprint types, Repository VCS types, Herald rule types, etc.

Today, we generally do this with radio buttons and a "Submit" button. This isn't terrible, but often it means users have to click twice (once on the radio; once on submit) when a single click would be sufficient. The radio click target can also be small.

In other cases, we have a container with a link and we'd like to link the entire container: notifications, the `/drydock/` console, etc. We'd like to just link the entire container, but this causes some problems:

  - It's not legal to link block eleements like `<a><div> ... </div></a>` and some browsers actually get upset about it.
  - We can `<a><span> ... </span></a>` instead, then turn the `<span>` into a block element with CSS -- and this sometimes works, but also has some drawbacks:
    - It's not great to do that for screenreaders, since the readable text in the link isn't necessarily very meaningful.
    - We can't have any other links inside the element (e.g., details or documentation).
  - We can `<form><button> ... </button></form>` instead, but this has its own set of problems:
    - You can't right-click to interact with a button in the same way you can with a link.
    - Also not great for screenreaders.

Instead, try adding a `linked-container` behavior which just means "when users click this element, pretend they clicked the first link inside it".

This gives us natural HTML (real, legal HTML with actual `<a>` tags) and good screenreader behavior, but allows the effective link target to be visually larger than just the link.

If no issues crop up with this, I'd plan to eventually use this technique in more places (Repositories, Herald, Almanac, Drydock, Notifications menu, etc).

Test Plan:
{F6053035}

  - Left-clicked and command-left-clicked the new JS fanciness, got sensible behaviors.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13222, T12588

Differential Revision: https://secure.phabricator.com/D19855
This commit is contained in:
epriestley 2018-12-07 06:04:07 -08:00
parent a6632f8c18
commit 68b1dee139
8 changed files with 123 additions and 45 deletions

View file

@ -9,7 +9,7 @@ return array(
'names' => array( 'names' => array(
'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.css' => 'e68cf1fa',
'conpherence.pkg.js' => '15191c65', 'conpherence.pkg.js' => '15191c65',
'core.pkg.css' => 'cff4ff6f', 'core.pkg.css' => '9d1148a4',
'core.pkg.js' => '4bde473b', 'core.pkg.js' => '4bde473b',
'differential.pkg.css' => '06dc617c', 'differential.pkg.css' => '06dc617c',
'differential.pkg.js' => 'ef0b989b', 'differential.pkg.js' => 'ef0b989b',
@ -127,7 +127,7 @@ return array(
'rsrc/css/phui/calendar/phui-calendar-list.css' => '576be600', 'rsrc/css/phui/calendar/phui-calendar-list.css' => '576be600',
'rsrc/css/phui/calendar/phui-calendar-month.css' => '21154caf', 'rsrc/css/phui/calendar/phui-calendar-month.css' => '21154caf',
'rsrc/css/phui/calendar/phui-calendar.css' => 'f1ddf11c', 'rsrc/css/phui/calendar/phui-calendar.css' => 'f1ddf11c',
'rsrc/css/phui/object-item/phui-oi-big-ui.css' => '628f59de', 'rsrc/css/phui/object-item/phui-oi-big-ui.css' => '7a7c22af',
'rsrc/css/phui/object-item/phui-oi-color.css' => 'cd2b9b77', 'rsrc/css/phui/object-item/phui-oi-color.css' => 'cd2b9b77',
'rsrc/css/phui/object-item/phui-oi-drag-ui.css' => '08f4ccc3', 'rsrc/css/phui/object-item/phui-oi-drag-ui.css' => '08f4ccc3',
'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '9d9685d6', 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '9d9685d6',
@ -469,6 +469,7 @@ return array(
'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0', 'rsrc/js/core/behavior-keyboard-shortcuts.js' => '01fca1f0',
'rsrc/js/core/behavior-lightbox-attachments.js' => '6b31879a', 'rsrc/js/core/behavior-lightbox-attachments.js' => '6b31879a',
'rsrc/js/core/behavior-line-linker.js' => '66a62306', 'rsrc/js/core/behavior-line-linker.js' => '66a62306',
'rsrc/js/core/behavior-linked-container.js' => '291da458',
'rsrc/js/core/behavior-more.js' => 'a80d0378', 'rsrc/js/core/behavior-more.js' => 'a80d0378',
'rsrc/js/core/behavior-object-selector.js' => '77c1f0b0', 'rsrc/js/core/behavior-object-selector.js' => '77c1f0b0',
'rsrc/js/core/behavior-oncopy.js' => '2926fff2', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2',
@ -616,6 +617,7 @@ return array(
'javelin-behavior-launch-icon-composer' => '48086888', 'javelin-behavior-launch-icon-composer' => '48086888',
'javelin-behavior-lightbox-attachments' => '6b31879a', 'javelin-behavior-lightbox-attachments' => '6b31879a',
'javelin-behavior-line-chart' => 'e4232876', 'javelin-behavior-line-chart' => 'e4232876',
'javelin-behavior-linked-container' => '291da458',
'javelin-behavior-maniphest-batch-selector' => 'ad54037e', 'javelin-behavior-maniphest-batch-selector' => 'ad54037e',
'javelin-behavior-maniphest-list-editor' => 'a9f88de2', 'javelin-behavior-maniphest-list-editor' => 'a9f88de2',
'javelin-behavior-maniphest-subpriority-editor' => '71237763', 'javelin-behavior-maniphest-subpriority-editor' => '71237763',
@ -832,7 +834,7 @@ return array(
'phui-lightbox-css' => '0a035e40', 'phui-lightbox-css' => '0a035e40',
'phui-list-view-css' => '38f8c9bd', 'phui-list-view-css' => '38f8c9bd',
'phui-object-box-css' => '9cff003c', 'phui-object-box-css' => '9cff003c',
'phui-oi-big-ui-css' => '628f59de', 'phui-oi-big-ui-css' => '7a7c22af',
'phui-oi-color-css' => 'cd2b9b77', 'phui-oi-color-css' => 'cd2b9b77',
'phui-oi-drag-ui-css' => '08f4ccc3', 'phui-oi-drag-ui-css' => '08f4ccc3',
'phui-oi-flush-ui-css' => '9d9685d6', 'phui-oi-flush-ui-css' => '9d9685d6',
@ -1027,6 +1029,10 @@ return array(
'javelin-uri', 'javelin-uri',
'phabricator-notification', 'phabricator-notification',
), ),
'291da458' => array(
'javelin-behavior',
'javelin-dom',
),
'2926fff2' => array( '2926fff2' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',
@ -1348,9 +1354,6 @@ return array(
'javelin-magical-init', 'javelin-magical-init',
'javelin-util', 'javelin-util',
), ),
'628f59de' => array(
'phui-oi-list-view-css',
),
'62dfea03' => array( '62dfea03' => array(
'javelin-install', 'javelin-install',
'javelin-util', 'javelin-util',
@ -1508,6 +1511,9 @@ return array(
'owners-path-editor', 'owners-path-editor',
'javelin-behavior', 'javelin-behavior',
), ),
'7a7c22af' => array(
'phui-oi-list-view-css',
),
'7cbe244b' => array( '7cbe244b' => array(
'javelin-install', 'javelin-install',
'javelin-util', 'javelin-util',

View file

@ -34,6 +34,7 @@ final class DrydockConsoleController extends DrydockController {
->setHeader(pht('Blueprints')) ->setHeader(pht('Blueprints'))
->setImageIcon('fa-map-o') ->setImageIcon('fa-map-o')
->setHref($this->getApplicationURI('blueprint/')) ->setHref($this->getApplicationURI('blueprint/'))
->setClickable(true)
->addAttribute( ->addAttribute(
pht( pht(
'Configure blueprints so Drydock can build resources, like '. 'Configure blueprints so Drydock can build resources, like '.
@ -44,6 +45,7 @@ final class DrydockConsoleController extends DrydockController {
->setHeader(pht('Resources')) ->setHeader(pht('Resources'))
->setImageIcon('fa-map') ->setImageIcon('fa-map')
->setHref($this->getApplicationURI('resource/')) ->setHref($this->getApplicationURI('resource/'))
->setClickable(true)
->addAttribute( ->addAttribute(
pht('View and manage resources Drydock has built, like hosts.'))); pht('View and manage resources Drydock has built, like hosts.')));
@ -52,6 +54,7 @@ final class DrydockConsoleController extends DrydockController {
->setHeader(pht('Leases')) ->setHeader(pht('Leases'))
->setImageIcon('fa-link') ->setImageIcon('fa-link')
->setHref($this->getApplicationURI('lease/')) ->setHref($this->getApplicationURI('lease/'))
->setClickable(true)
->addAttribute(pht('Manage leases on resources.'))); ->addAttribute(pht('Manage leases on resources.')));
$menu->addItem( $menu->addItem(
@ -59,6 +62,7 @@ final class DrydockConsoleController extends DrydockController {
->setHeader(pht('Repository Operations')) ->setHeader(pht('Repository Operations'))
->setImageIcon('fa-fighter-jet') ->setImageIcon('fa-fighter-jet')
->setHref($this->getApplicationURI('operation/')) ->setHref($this->getApplicationURI('operation/'))
->setClickable(true)
->addAttribute(pht('Review the repository operation queue.'))); ->addAttribute(pht('Review the repository operation queue.')));
$crumbs = $this->buildApplicationCrumbs(); $crumbs = $this->buildApplicationCrumbs();

View file

@ -37,39 +37,34 @@ final class ManiphestTaskSubtaskController
->addCancelButton($cancel_uri, pht('Close')); ->addCancelButton($cancel_uri, pht('Close'));
} }
if ($request->isFormPost()) { $menu = id(new PHUIObjectItemListView())
$form_key = $request->getStr('formKey'); ->setUser($viewer)
if (isset($subtype_options[$form_key])) { ->setBig(true)
$subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/")) ->setFlush(true);
->setQueryParam('parent', $id)
->setQueryParam('template', $id)
->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus());
$subtask_uri = $this->getApplicationURI($subtask_uri);
return id(new AphrontRedirectResponse()) foreach ($subtype_options as $form_key => $subtype_form) {
->setURI($subtask_uri); $subtype_key = $subtype_form->getSubtype();
} $subtype = $subtype_map->getSubtype($subtype_key);
$subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/"))
->setQueryParam('parent', $id)
->setQueryParam('template', $id)
->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus());
$subtask_uri = $this->getApplicationURI($subtask_uri);
$item = id(new PHUIObjectItemView())
->setHeader($subtype_form->getDisplayName())
->setHref($subtask_uri)
->setClickable(true)
->setImageIcon($subtype->newIconView())
->addAttribute($subtype->getName());
$menu->addItem($item);
} }
$control = id(new AphrontFormRadioButtonControl())
->setName('formKey')
->setLabel(pht('Subtype'));
foreach ($subtype_options as $key => $subtype_form) {
$control->addButton(
$key,
$subtype_form->getDisplayName(),
null);
}
$form = id(new AphrontFormView())
->setViewer($viewer)
->appendControl($control);
return $this->newDialog() return $this->newDialog()
->setTitle(pht('Choose Subtype')) ->setTitle(pht('Choose Subtype'))
->appendForm($form) ->appendChild($menu)
->addSubmitButton(pht('Continue'))
->addCancelButton($cancel_uri); ->addCancelButton($cancel_uri);
} }

View file

@ -239,4 +239,9 @@ final class PhabricatorEditEngineSubtype
return new PhabricatorEditEngineSubtypeMap($map); return new PhabricatorEditEngineSubtypeMap($map);
} }
public function newIconView() {
return id(new PHUIIconView())
->setIcon($this->getIcon(), $this->getColor());
}
} }

View file

@ -404,7 +404,7 @@ final class AphrontDialogView
$header), $header),
phutil_tag('div', phutil_tag('div',
array( array(
'class' => 'aphront-dialog-body phabricator-remarkup grouped', 'class' => 'aphront-dialog-body grouped',
), ),
$children), $children),
$tail, $tail,

View file

@ -28,6 +28,7 @@ final class PHUIObjectItemView extends AphrontTagView {
private $sideColumn; private $sideColumn;
private $coverImage; private $coverImage;
private $description; private $description;
private $clickable;
private $selectableName; private $selectableName;
private $selectableValue; private $selectableValue;
@ -179,6 +180,15 @@ final class PHUIObjectItemView extends AphrontTagView {
return $this; return $this;
} }
public function setClickable($clickable) {
$this->clickable = $clickable;
return $this;
}
public function getClickable() {
return $this->clickable;
}
public function setEpoch($epoch) { public function setEpoch($epoch) {
$date = phabricator_datetime($epoch, $this->getUser()); $date = phabricator_datetime($epoch, $this->getUser());
$this->addIcon('none', $date); $this->addIcon('none', $date);
@ -332,6 +342,13 @@ final class PHUIObjectItemView extends AphrontTagView {
$item_classes[] = 'phui-oi-with-image-icon'; $item_classes[] = 'phui-oi-with-image-icon';
} }
if ($this->getClickable()) {
Javelin::initBehavior('linked-container');
$item_classes[] = 'phui-oi-linked-container';
$sigils[] = 'linked-container';
}
return array( return array(
'class' => $item_classes, 'class' => $item_classes,
'sigil' => $sigils, 'sigil' => $sigils,
@ -443,15 +460,6 @@ final class PHUIObjectItemView extends AphrontTagView {
), ),
$spec['label']); $spec['label']);
if (isset($spec['attributes']['href'])) {
$icon_href = phutil_tag(
'a',
array('href' => $spec['attributes']['href']),
array($icon, $label));
} else {
$icon_href = array($icon, $label);
}
$classes = array(); $classes = array();
$classes[] = 'phui-oi-icon'; $classes[] = 'phui-oi-icon';
if (isset($spec['attributes']['class'])) { if (isset($spec['attributes']['class'])) {
@ -463,7 +471,7 @@ final class PHUIObjectItemView extends AphrontTagView {
array( array(
'class' => implode(' ', $classes), 'class' => implode(' ', $classes),
), ),
$icon_href); $icon);
} }
$icons[] = phutil_tag( $icons[] = phutil_tag(

View file

@ -59,3 +59,16 @@
background-color: {$hoverblue}; background-color: {$hoverblue};
border-radius: 3px; border-radius: 3px;
} }
.device-desktop .phui-oi-linked-container {
cursor: pointer;
}
.device-desktop .phui-oi-linked-container:hover {
background-color: {$hoverblue};
border-radius: 3px;
}
.device-desktop .phui-oi-linked-container a:hover {
text-decoration: none;
}

View file

@ -0,0 +1,47 @@
/**
* @provides javelin-behavior-linked-container
* @requires javelin-behavior javelin-dom
*/
JX.behavior('linked-container', function() {
JX.Stratcom.listen(
'click',
'linked-container',
function(e) {
// If the user clicked some link inside the container, bail out and just
// click the link.
if (e.getNode('tag:a')) {
return;
}
// If this is some sort of unusual click, bail out. Note that we'll
// handle "Left Click" and "Command + Left Click" differently, below.
if (!e.isLeftButton()) {
return;
}
var container = e.getNode('linked-container');
// Find the first link in the container. We're going to pretend the user
// clicked it.
var link = JX.DOM.scry(container, 'a')[0];
if (!link) {
return;
}
// If the click is a "Command + Left Click", change the target of the
// link so we open it in a new tab.
var is_command = !!e.getRawEvent().metaKey;
if (is_command) {
var old_target = link.target;
link.target = '_blank';
link.click();
link.target = old_target;
} else {
link.click();
}
});
});