Make date control include times

Summary:
See discussion in T404. Basically, the problem with date-only controls is that they may behave unpredictably in the presence of timezones. When you say "This needs to be done by Oct 23", you probably mean "Oct 23 5PM PST" or something like that, but someone in China may see the "Oct 24" and hit the deadline in good faith but be 10 hours too late. T404 has more discussion and examples. There are ways to fake this, but they get more complicated if the guy in China needs to move the date forward 24 hours.

I think the best solution to this is to not have date-only controls, and always display the time. This makes it absolutley unambiguous what something means, because the guy in the US will set "Oct 23 5PM" and the guy in China will see that accurately in local time.

The downside is that it's slightly more visual clutter and work for the user to specify things precisely, but I added some hints (start/end of day, start/end of business) that will hopefully let us pick the right default in most cases.

Test Plan:
Set some dates.

{F21956}

This has a couple of edge case issues on resize and some not-so-edge-case issues on mobile, but should be good to build T407 on without API changes.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T404, T407

Differential Revision: https://secure.phabricator.com/D3793

src/applications/uiexample/examples/PhabricatorFormExample.php

@@ -30,24 +30,29 @@ final class PhabricatorFormExample extends PhabricatorUIExample {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    $date = id(new AphrontFormDateControl())
+    $start_time = id(new AphrontFormDateControl())
       ->setUser($user)
-      ->setName('date')
-      ->setLabel('Date');
+      ->setName('start')
+      ->setLabel('Start')
+      ->setInitialTime(AphrontFormDateControl::TIME_START_OF_BUSINESS);
+    $start_value = $start_time->readValueFromRequest($request);
 
-    $date->readValueFromRequest($request);
+    $end_time = id(new AphrontFormDateControl())
+      ->setUser($user)
+      ->setName('end')
+      ->setLabel('End')
+      ->setInitialTime(AphrontFormDateControl::TIME_END_OF_BUSINESS);
+    $end_value = $end_time->readValueFromRequest($request);
 
     $form = id(new AphrontFormView())
       ->setUser($user)
-      ->appendChild($date)
+      ->setFlexible(true)
+      ->appendChild($start_time)
+      ->appendChild($end_time)
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue('Submit'));
 
-    $panel = new AphrontPanelView();
-    $panel->setHeader('Form');
-    $panel->appendChild($form);
-
-    return $panel;
+    return $form;
   }
 }

src/view/form/control/AphrontFormDateControl.php

@@ -19,55 +19,128 @@
 final class AphrontFormDateControl extends AphrontFormControl {
 
   private $user;
+  private $initialTime;
 
-  public function setUser($user) {
+  private $valueDay;
+  private $valueMonth;
+  private $valueYear;
+  private $valueTime;
+
+  const TIME_START_OF_DAY         = 'start-of-day';
+  const TIME_END_OF_DAY           = 'end-of-day';
+  const TIME_START_OF_BUSINESS    = 'start-of-business';
+  const TIME_END_OF_BUSINESS      = 'end-of-business';
+
+  public function setUser(PhabricatorUser $user) {
     $this->user = $user;
     return $this;
   }
 
+  public function setInitialTime($time) {
+    $this->initialTime = $time;
+    return $this;
+  }
+
   public function readValueFromRequest(AphrontRequest $request) {
+    $user = $this->user;
+    if (!$this->user) {
+      throw new Exception(
+        "Call setUser() before readValueFromRequest()!");
+    }
+
+    $user_zone = $user->getTimezoneIdentifier();
+    $zone = new DateTimeZone($user_zone);
 
     $day = $request->getInt($this->getDayInputName());
     $month = $request->getInt($this->getMonthInputName());
     $year = $request->getInt($this->getYearInputName());
+    $time = $request->getStr($this->getTimeInputName());
 
     $err = $this->getError();
 
-    if ($day || $month || $year) {
+    if ($day || $month || $year || $time) {
+      $this->valueDay = $day;
+      $this->valueMonth = $month;
+      $this->valueYear = $year;
+      $this->valueTime = $time;
 
       // Assume invalid.
       $err = 'Invalid';
 
-      $tz = new DateTimeZone('UTC');
       try {
-        $date = new DateTime("{$year}-{$month}-{$day} 12:00:00 AM", $tz);
-        $value = $date->format('Y-m-d');
-        if ($value) {
-          $this->setValue($value);
-          $err = null;
-        }
+        $date = new DateTime("{$year}-{$month}-{$day} {$time}", $zone);
+        $value = $date->format('U');
       } catch (Exception $ex) {
-        // Ignore, already handled.
+        $value = null;
+      }
+
+      if ($value) {
+        $this->setValue($value);
+        $err = null;
+      } else {
+        $this->setValue(null);
+      }
+    } else {
+      // TODO: We could eventually allow these to be customized per install or
+      // per user or both, but let's wait and see.
+      switch ($this->initialTime) {
+        case self::TIME_START_OF_DAY:
+        default:
+          $time = '12:00 AM';
+          break;
+        case self::TIME_START_OF_BUSINESS:
+          $time = '9:00 AM';
+          break;
+        case self::TIME_END_OF_BUSINESS:
+          $time = '5:00 PM';
+          break;
+        case self::TIME_END_OF_DAY:
+          $time = '11:59 PM';
+          break;
+      }
+
+      $today = $this->formatTime(time(), 'Y-m-d');
+      try {
+        $date = new DateTime("{$today} {$time}", $zone);
+        $value = $date->format('U');
+      } catch (Exception $ex) {
+        $value = null;
+      }
+
+      if ($value) {
+        $this->setValue($value);
+      } else {
+        $this->setValue(null);
       }
     }
 
     $this->setError($err);
 
-    return $err;
+    return $this->getValue();
   }
 
-  public function getValue() {
-    if (!parent::getValue()) {
-      $this->setValue($this->formatTime(time(), 'Y-m-d'));
-    }
-    return parent::getValue();
-  }
-
-
   protected function getCustomControlClass() {
     return 'aphront-form-control-date';
   }
 
+  public function setValue($epoch) {
+    $result = parent::setValue($epoch);
+
+    if ($epoch === null) {
+      return;
+    }
+
+    $readable = $this->formatTime($epoch, 'Y!m!d!g:i A');
+    $readable = explode('!', $readable, 4);
+
+    $this->valueYear  = $readable[0];
+    $this->valueMonth = $readable[1];
+    $this->valueDay   = $readable[2];
+    $this->valueTime  = $readable[3];
+
+    return $result;
+  }
+
   private function getMinYear() {
     $cur_year = $this->formatTime(
       time(),
@@ -87,15 +160,19 @@ final class AphrontFormDateControl extends AphrontFormControl {
   }
 
   private function getDayInputValue() {
-    return (int)idx(explode('-', $this->getValue()), 2);
+    return $this->valueDay;
   }
 
   private function getMonthInputValue() {
-    return (int)idx(explode('-', $this->getValue()), 1);
+    return $this->valueMonth;
   }
 
   private function getYearInputValue() {
-    return (int)idx(explode('-', $this->getValue()), 0);
+    return $this->valueYear;
+  }
+
+  private function getTimeInputValue() {
+    return $this->valueTime;
   }
 
   private function formatTime($epoch, $fmt) {
@@ -117,6 +194,10 @@ final class AphrontFormDateControl extends AphrontFormControl {
     return $this->getName().'_y';
   }
 
+  private function getTimeInputName() {
+    return $this->getName().'_t';
+  }
+
   protected function renderInput() {
     $min_year = $this->getMinYear();
     $max_year = $this->getMaxYear();
@@ -175,19 +256,24 @@ final class AphrontFormDateControl extends AphrontFormControl {
       ),
       '');
 
-    $id = celerity_generate_unique_node_id();
-
-    Javelin::initBehavior(
-      'fancy-datepicker',
+    $time_sel = phutil_render_tag(
+      'input',
       array(
-        'root' => $id,
-      ));
+        'name'  => $this->getTimeInputName(),
+        'sigil' => 'time-input',
+        'value' => $this->getTimeInputValue(),
+        'type'  => 'text',
+        'class' => 'aphront-form-date-time-input',
+      ),
+      '');
+
+    Javelin::initBehavior('fancy-datepicker', array());
 
     return javelin_render_tag(
       'div',
       array(
-        'id' => $id,
         'class' => 'aphront-form-date-container',
+        'sigil' => 'phabricator-date-control',
       ),
       self::renderSingleView(
         array(
@@ -195,6 +281,7 @@ final class AphrontFormDateControl extends AphrontFormControl {
           $months_sel,
           $years_sel,
           $cal_icon,
+          $time_sel,
         )));
   }
 

webroot/rsrc/css/aphront/form-view.css

@@ -194,14 +194,13 @@ table.aphront-form-control-checkbox-layout th {
 }
 
 .calendar-button {
-  padding: 11px;
-  right: -30px;
-  top: -3px;
-
+  display: inline;
   background: url(/rsrc/image/icon/fatcow/calendar_edit.png)
     no-repeat center center;
-  z-index: 2;
-  position: absolute;
+  padding: 8px 12px;
+  margin: 2px 8px 2px 2px;
+  position: relative;
+  z-index: 8;
   border: 1px solid transparent;
 }
 
@@ -210,16 +209,20 @@ table.aphront-form-control-checkbox-layout th {
   display: inline;
 }
 
-.aphront-form-date-container select{
+.aphront-form-date-container select {
   margin: 2px;
+  display: inline;
+}
+
+.aphront-form-date-container input.aphront-form-date-time-input {
+  width: 7em;
+  display: inline;
 }
 
 .fancy-datepicker {
   position: absolute;
-  top: -10px;
-  right: -8px;
   width: 240px;
-  padding-bottom: 6em;
+  z-index: 7;
 }
 
 .fancy-datepicker-core {

webroot/rsrc/js/application/core/behavior-fancy-datepicker.js

@@ -3,11 +3,15 @@
  * @requires javelin-behavior
  *           javelin-util
  *           javelin-dom
+ *           javelin-stratcom
+ *           javelin-vector
  */
 
 JX.behavior('fancy-datepicker', function(config) {
 
   var picker;
+  var button;
+  var root;
 
   var value_y;
   var value_m;
@@ -20,15 +24,32 @@ JX.behavior('fancy-datepicker', function(config) {
     // without writing the change.
 
     if (picker) {
-      onclose(e);
-      return;
+      if (root == e.getNode('phabricator-date-control')) {
+        // If the user clicked the same control, just close it.
+        onclose(e);
+        return;
+      } else {
+        // If the user clicked a different control, close the old one but then
+        // open the new one.
+        onclose(e);
+      }
     }
 
+
+    root = e.getNode('phabricator-date-control');
+
     picker = JX.$N(
       'div',
-      {className: 'fancy-datepicker'},
+      {className: 'fancy-datepicker', sigil: 'phabricator-datepicker'},
       JX.$N('div', {className: 'fancy-datepicker-core'}));
-    root.appendChild(picker);
+    document.body.appendChild(picker);
+
+    var button = e.getNode('calendar-button');
+    var p = JX.$V(button);
+    var d = JX.Vector.getDim(picker);
+
+    picker.style.left = (p.x - d.x + 2) + 'px';
+    picker.style.top = (p.y - 10) + 'px';
 
     JX.DOM.alterClass(root, 'picker-open', true);
 
@@ -45,6 +66,8 @@ JX.behavior('fancy-datepicker', function(config) {
     picker = null;
     JX.DOM.alterClass(root, 'picker-open', false);
     e.kill();
+
+    root = null;
   };
 
   var get_inputs = function() {
@@ -176,18 +199,11 @@ JX.behavior('fancy-datepicker', function(config) {
   };
 
 
-  var root = JX.$(config.root);
+  JX.Stratcom.listen('click', 'calendar-button', onopen);
 
-  JX.DOM.listen(
-    root,
+  JX.Stratcom.listen(
     'click',
-    'calendar-button',
-    onopen);
-
-  JX.DOM.listen(
-    root,
-    'click',
-    'tag:td',
+    ['phabricator-datepicker', 'tag:td'],
     function(e) {
       e.kill();