From b584834b193413806e91a03d0e94508f3bcc34e5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 24 Aug 2018 10:18:28 -0700
Subject: [PATCH] In Differential: when the file tree is enabled, default to
 the "History" tab instead of "Files"

Summary:
Ref T13187. See PHI811. If the file tree is enabled and visible, set the default tab to "History".

  - This is a bit magic.
  - It won't work entirely on mobile (we can't tell that you're on mobile on the server, so we'll pick the "History" tab even though the file tree isn't actually visible on your device).
  - There's no corresponding logic in Diffusion. Diffusion doesn't have the same tab layout, but this makes things somewhat inconsistent.

So I don't love this, but we can try it and see if it's confusing or helpful on the balance.

Test Plan: With filetree on and off, reloaded revisions. Saw appropriate tab selected by default.

Reviewers: amckinley

Maniphest Tasks: T13187

Differential Revision: https://secure.phabricator.com/D19601
---
 .../DifferentialRevisionViewController.php    | 46 +++++++++++--------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index 33440e535f..b809a8ee80 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -463,7 +463,7 @@ final class DifferentialRevisionViewController
       }
     }
 
-    $tab_group = id(new PHUITabGroupView());
+    $tab_group = new PHUITabGroupView();
 
     if ($toc_view) {
       $tab_group->addTab(
@@ -473,17 +473,30 @@ final class DifferentialRevisionViewController
           ->appendChild($toc_view));
     }
 
-    $tab_group
-      ->addTab(
-        id(new PHUITabView())
-          ->setName(pht('History'))
-          ->setKey('history')
-          ->appendChild($history))
-      ->addTab(
-        id(new PHUITabView())
-          ->setName(pht('Commits'))
-          ->setKey('commits')
-          ->appendChild($local_table));
+    $tab_group->addTab(
+      id(new PHUITabView())
+        ->setName(pht('History'))
+        ->setKey('history')
+        ->appendChild($history));
+
+    $filetree_on = $viewer->compareUserSetting(
+      PhabricatorShowFiletreeSetting::SETTINGKEY,
+      PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE);
+
+    $collapsed_key = PhabricatorFiletreeVisibleSetting::SETTINGKEY;
+    $filetree_collapsed = (bool)$viewer->getUserSetting($collapsed_key);
+
+    // See PHI811. If the viewer has the file tree on, the files tab with the
+    // table of contents is redundant, so default to the "History" tab instead.
+    if ($filetree_on && !$filetree_collapsed) {
+      $tab_group->selectTab('history');
+    }
+
+    $tab_group->addTab(
+      id(new PHUITabView())
+        ->setName(pht('Commits'))
+        ->setKey('commits')
+        ->appendChild($local_table));
 
     $stack_graph = id(new DifferentialRevisionGraph())
       ->setViewer($viewer)
@@ -601,22 +614,15 @@ final class DifferentialRevisionViewController
     $crumbs->addTextCrumb($monogram);
     $crumbs->setBorder(true);
 
-    $filetree_on = $viewer->compareUserSetting(
-      PhabricatorShowFiletreeSetting::SETTINGKEY,
-      PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE);
-
     $nav = null;
     if ($filetree_on && !$this->isVeryLargeDiff()) {
-      $collapsed_key = PhabricatorFiletreeVisibleSetting::SETTINGKEY;
-      $collapsed_value = $viewer->getUserSetting($collapsed_key);
-
       $width_key = PhabricatorFiletreeWidthSetting::SETTINGKEY;
       $width_value = $viewer->getUserSetting($width_key);
 
       $nav = id(new DifferentialChangesetFileTreeSideNavBuilder())
         ->setTitle($monogram)
         ->setBaseURI(new PhutilURI($revision->getURI()))
-        ->setCollapsed((bool)$collapsed_value)
+        ->setCollapsed($filetree_collapsed)
         ->setWidth((int)$width_value)
         ->build($changesets);
     }