From e5a16819031e4dd1a7914f7bc471a442ba70706a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 May 2019 08:02:39 -0700
Subject: [PATCH] Render timezone names more readably, with spaces rather than
 underscores ("America/Los Angeles", not "America/Los_Angeles").

Summary:
See downstream <https://phabricator.wikimedia.org/T902>. Currently, timezones are rendered with their raw internal names (like `America/Los_Angeles`) which include underscores.

Replacing underscores with spaces is a more human-readable (and perhaps meaningfully better for things like screen readers, although this is pure speculation).

There's some vague argument against this, like "administrators may need to set a raw internal value in `phabricator.timezone` and this could mislead them", but we already give a pretty good error message if you do this and could improve hinting if necessary.

Test Plan: Viewed timezone list in {nav Settings} and the timezone "reconcile" dialog, saw a more-readable "Los Angeles".

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20559
---
 .../PhabricatorSettingsTimezoneController.php          |  3 ++-
 .../settings/setting/PhabricatorTimezoneSetting.php    |  9 ++++++++-
 src/infrastructure/time/PhabricatorTime.php            | 10 ++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
index 6a0ba19d03..8ae2704161 100644
--- a/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
+++ b/src/applications/settings/controller/PhabricatorSettingsTimezoneController.php
@@ -20,7 +20,8 @@ final class PhabricatorSettingsTimezoneController
       $zone = new DateTimeZone($identifier);
       $offset = -($zone->getOffset($now) / 60);
       if ($offset == $client_offset) {
-        $options[$identifier] = $identifier;
+        $name = PhabricatorTime::getTimezoneDisplayName($identifier);
+        $options[$identifier] = $name;
       }
     }
 
diff --git a/src/applications/settings/setting/PhabricatorTimezoneSetting.php b/src/applications/settings/setting/PhabricatorTimezoneSetting.php
index 52fce77428..207ad95f62 100644
--- a/src/applications/settings/setting/PhabricatorTimezoneSetting.php
+++ b/src/applications/settings/setting/PhabricatorTimezoneSetting.php
@@ -81,9 +81,16 @@ final class PhabricatorTimezoneSetting
       }
 
       sort($group);
+
+      $group_map = array();
+      foreach ($group as $identifier) {
+        $name = PhabricatorTime::getTimezoneDisplayName($identifier);
+        $group_map[$identifier] = $name;
+      }
+
       $option_groups[] = array(
         'label' => $label,
-        'options' => array_fuse($group),
+        'options' => $group_map,
       );
     }
 
diff --git a/src/infrastructure/time/PhabricatorTime.php b/src/infrastructure/time/PhabricatorTime.php
index b221285d13..67378ca8df 100644
--- a/src/infrastructure/time/PhabricatorTime.php
+++ b/src/infrastructure/time/PhabricatorTime.php
@@ -78,4 +78,14 @@ final class PhabricatorTime extends Phobject {
     return $datetime;
   }
 
+  public static function getTimezoneDisplayName($raw_identifier) {
+
+    // Internal identifiers have names like "America/Los_Angeles", but this is
+    // just an implementation detail and we can render them in a more human
+    // readable format with spaces.
+    $name = str_replace('_', ' ', $raw_identifier);
+
+    return $name;
+  }
+
 }