Simplify Maniphest button filter toggle things

Summary:
Although these filters work pretty well, you still end up doing a double take
sometimes. Make the behavior simpler and more consistent by adding an "All"
button to "Open / Closed" so all three rows behave the same way (before, the top
row was toggleable but the other rows were select-only-one).

I played around with the styles a little bit too to try to make the selected
state more obvious.

sandra/anjali, let me know if this is good enough once it lands or if I should
go further in playing around with the styles and making it more clear.

Test Plan:
Filtered tasks with the various filter buttons, verified the task list
accurately represented the filters.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, sandra, aran, epriestley, tuomaspelkonen
Differential Revision: 364

src/__celerity_resource_map__.php

@@ -884,7 +884,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-core-buttons-css' =>
   array(
-    'uri' => '/res/f797baf5/rsrc/css/core/buttons.css',
+    'uri' => '/res/3059cf79/rsrc/css/core/buttons.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -1048,7 +1048,7 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/613cf273/differential.pkg.css',
       'type' => 'css',
     ),
-    'a6102fe7' =>
+    'c9c3eee2' =>
     array (
       'name' => 'core.pkg.css',
       'symbols' =>
@@ -1069,7 +1069,7 @@ celerity_register_resource_map(array(
         13 => 'phabricator-remarkup-css',
         14 => 'syntax-highlighting-css',
       ),
-      'uri' => '/res/pkg/a6102fe7/core.pkg.css',
+      'uri' => '/res/pkg/c9c3eee2/core.pkg.css',
       'type' => 'css',
     ),
     'db95a6d0' =>
@@ -1108,15 +1108,15 @@ celerity_register_resource_map(array(
   ),
   'reverse' =>
   array (
-    'aphront-crumbs-view-css' => 'a6102fe7',
+    'aphront-crumbs-view-css' => 'c9c3eee2',
-    'aphront-dialog-view-css' => 'a6102fe7',
+    'aphront-dialog-view-css' => 'c9c3eee2',
-    'aphront-form-view-css' => 'a6102fe7',
+    'aphront-form-view-css' => 'c9c3eee2',
-    'aphront-list-filter-view-css' => 'a6102fe7',
+    'aphront-list-filter-view-css' => 'c9c3eee2',
-    'aphront-panel-view-css' => 'a6102fe7',
+    'aphront-panel-view-css' => 'c9c3eee2',
-    'aphront-side-nav-view-css' => 'a6102fe7',
+    'aphront-side-nav-view-css' => 'c9c3eee2',
-    'aphront-table-view-css' => 'a6102fe7',
+    'aphront-table-view-css' => 'c9c3eee2',
-    'aphront-tokenizer-control-css' => 'a6102fe7',
+    'aphront-tokenizer-control-css' => 'c9c3eee2',
-    'aphront-typeahead-control-css' => 'a6102fe7',
+    'aphront-typeahead-control-css' => 'c9c3eee2',
     'differential-changeset-view-css' => '613cf273',
     'differential-core-view-css' => '613cf273',
     'differential-revision-add-comment-css' => '613cf273',
@@ -1151,11 +1151,11 @@ celerity_register_resource_map(array(
     'javelin-util' => 'db95a6d0',
     'javelin-vector' => 'db95a6d0',
     'javelin-workflow' => '122a6b6d',
-    'phabricator-core-buttons-css' => 'a6102fe7',
+    'phabricator-core-buttons-css' => 'c9c3eee2',
-    'phabricator-core-css' => 'a6102fe7',
+    'phabricator-core-css' => 'c9c3eee2',
-    'phabricator-directory-css' => 'a6102fe7',
+    'phabricator-directory-css' => 'c9c3eee2',
-    'phabricator-remarkup-css' => 'a6102fe7',
+    'phabricator-remarkup-css' => 'c9c3eee2',
-    'phabricator-standard-page-view' => 'a6102fe7',
+    'phabricator-standard-page-view' => 'c9c3eee2',
-    'syntax-highlighting-css' => 'a6102fe7',
+    'syntax-highlighting-css' => 'c9c3eee2',
   ),
 ));

src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php

@@ -301,73 +301,26 @@ class ManiphestTaskListController extends ManiphestController {
   public function renderStatusLinks() {
     $request = $this->getRequest();
 
-    $sel = array(
+    $statuses = array(
-      'open'   => false,
+      'o'   => array('open' => true),
-      'closed' => false,
+      'c'   => array('closed' => true),
+      'oc'  => array('open' => true, 'closed' => true),
     );
 
     $status = $request->getStr('s');
-    if ($status == 'c') {
+    if (empty($statuses[$status])) {
-      $sel['closed'] = true;
+      $status = 'o';
-    } else if ($status == 'oc') {
-      $sel['closed'] = true;
-      $sel['open'] = true;
-    } else {
-      $sel['open'] = true;
     }
 
-    $just_one = (count(array_filter($sel)) == 1);
-
-    $flag_map = array(
-      'Open'    => 'open',
-      'Closed'  => 'closed',
-    );
     $button_names = array(
       'Open'    => 'o',
       'Closed'  => 'c',
+      'All'     => 'oc',
     );
 
-    $base_flags = array();
+    $status_links = $this->renderFilterLinks($button_names, $status, 's');
-    foreach ($flag_map as $name => $key) {
-      $base_flags[$button_names[$name]] = $sel[$key];
-    }
 
-    foreach ($button_names as $name => $letter) {
+    return array($statuses[$status], $status_links);
-      $flags = $base_flags;
-      $flags[$letter] = !$flags[$letter];
-      $button_names[$name] = implode('', array_keys(array_filter($flags)));
-    }
-
-    $uri = $request->getRequestURI();
-
-    $links = array();
-    foreach ($button_names as $name => $value) {
-      $selected = $sel[$flag_map[$name]];
-      $fixed = ($selected && $just_one);
-
-      $more = null;
-      if ($fixed) {
-        $href = null;
-        $more .= ' toggle-fixed';
-      } else {
-        $href = $uri->alter('s', $value);
-      }
-
-      if ($selected) {
-        $more .= ' toggle-selected';
-      }
-
-      $links[] = phutil_render_tag(
-        'a',
-        array(
-          'href'  => $href,
-          'class' => 'toggle'.$more,
-        ),
-        $name);
-    }
-    $status_links = implode("\n", $links);
-
-    return array($sel, $status_links);
   }
 
   public function renderOrderLinks() {
@@ -390,26 +343,7 @@ class ManiphestTaskListController extends ManiphestController {
       'Created'   => 'c',
     );
 
-    $uri = $request->getRequestURI();
+    $order_links = $this->renderFilterLinks($order_names, $order, 'o');
-
-    $links = array();
-    foreach ($order_names as $name => $param_key) {
-      if ($param_key == $order) {
-        $more = ' toggle-selected toggle-fixed';
-        $href = null;
-      } else {
-        $more = null;
-        $href = $uri->alter('o', $param_key);
-      }
-      $links[] = phutil_render_tag(
-        'a',
-        array(
-          'class' => 'toggle'.$more,
-          'href'  => $href,
-        ),
-        $name);
-    }
-    $order_links = implode("\n", $links);
 
     return array($order_by, $order_links);
   }
@@ -436,16 +370,23 @@ class ManiphestTaskListController extends ManiphestController {
       'None'      => 'n',
     );
 
+    $group_links = $this->renderFilterLinks($group_names, $group, 'g');
+
+    return array($group_by, $group_links);
+  }
+
+  private function renderFilterLinks($filter_map, $selected, $uri_param) {
+    $request = $this->getRequest();
     $uri = $request->getRequestURI();
 
     $links = array();
-    foreach ($group_names as $name => $param_key) {
+    foreach ($filter_map as $name => $value) {
-      if ($param_key == $group) {
+      if ($value == $selected) {
         $more = ' toggle-selected toggle-fixed';
         $href = null;
       } else {
         $more = null;
-        $href = $uri->alter('g', $param_key);
+        $href = $uri->alter($uri_param, $value);
       }
       $links[] = phutil_render_tag(
         'a',
@@ -455,10 +396,7 @@ class ManiphestTaskListController extends ManiphestController {
         ),
         $name);
     }
-    $group_links = implode("\n", $links);
+    return implode("\n", $links);
-
-    return array($group_by, $group_links);
   }
 
-
 }

webroot/rsrc/css/core/buttons.css

@@ -161,7 +161,7 @@ a.toggle {
   padding: 2px 6px 3px;
   font-weight: bold;
   margin-right: 4px;
-  color: #333333;
+  color: #777777;
   border-radius: 8px;
   -moz-border-radius: 8px;
   -webkit-border-radius: 8px;