Make the paste creation form work on phones

Summary: This is the first time I've ever had CSS actually work like it promises it does (i.e., markup the "right" way and then you don't have to change the markup later). Since I laboriously laid this whole thing out with <divs> originally, I was able to just override some of the styles and make the layout reasonable for devices. The only differences for existing forms are: - No colon after labels (looks cleaner anyway). - Non-error required text is no longer a red star but a the grey word "Required" (this is clearer). Test Plan: Viewed paste form on a phone. Viewed ~20 other forms on the site to verify that I didn't break anything. Reviewers: btrahan, chad, vrana Reviewed By: btrahan CC: aran Maniphest Tasks: T1569 Differential Revision: https://secure.phabricator.com/D3298
2024-11-26 16:52:41 +01:00 · 2012-08-15 14:15:12 -07:00 · 2012-08-15 14:15:12 -07:00 · fa5a5a0a12
commit fa5a5a0a12
parent f5e738b7ca
6 changed files with 133 additions and 45 deletions
--- a/src/__celerity_resource_map__.php
+++ b/src/__celerity_resource_map__.php
@ -534,7 +534,7 @@ celerity_register_resource_map(array(
  ),
  'aphront-form-view-css' =>
  array(
-    'uri' => '/res/b60e1526/rsrc/css/aphront/form-view.css',
+    'uri' => '/res/2f8cf4e3/rsrc/css/aphront/form-view.css',
    'type' => 'css',
    'requires' =>
    array(
@ -2343,6 +2343,15 @@ celerity_register_resource_map(array(
    ),
    'disk' => '/rsrc/css/application/flag/flag.css',
  ),
+  'phabricator-form-view-css' =>
+  array(
+    'uri' => '/res/7609ca21/rsrc/css/layout/phabricator-form-view.css',
+    'type' => 'css',
+    'requires' =>
+    array(
+    ),
+    'disk' => '/rsrc/css/layout/phabricator-form-view.css',
+  ),
  'phabricator-header-view-css' =>
  array(
    'uri' => '/res/c89cc14d/rsrc/css/layout/phabricator-header-view.css',
@ -2876,7 +2885,7 @@ celerity_register_resource_map(array(
 ), array(
  'packages' =>
  array(
-    'dd49ed34' =>
+    'e3da34c9' =>
    array(
      'name' => 'core.pkg.css',
      'symbols' =>
@ -2905,7 +2914,7 @@ celerity_register_resource_map(array(
        21 => 'phabricator-flag-css',
        22 => 'aphront-error-view-css',
      ),
-      'uri' => '/res/pkg/dd49ed34/core.pkg.css',
+      'uri' => '/res/pkg/e3da34c9/core.pkg.css',
      'type' => 'css',
    ),
    '971b021e' =>
@ -3072,20 +3081,20 @@ celerity_register_resource_map(array(
  'reverse' =>
  array(
    'aphront-attached-file-view-css' => '7839ae2d',
-    'aphront-crumbs-view-css' => 'dd49ed34',
-    'aphront-dialog-view-css' => 'dd49ed34',
-    'aphront-error-view-css' => 'dd49ed34',
-    'aphront-form-view-css' => 'dd49ed34',
+    'aphront-crumbs-view-css' => 'e3da34c9',
+    'aphront-dialog-view-css' => 'e3da34c9',
+    'aphront-error-view-css' => 'e3da34c9',
+    'aphront-form-view-css' => 'e3da34c9',
    'aphront-headsup-action-list-view-css' => '19ebcc79',
-    'aphront-headsup-view-css' => 'dd49ed34',
-    'aphront-list-filter-view-css' => 'dd49ed34',
-    'aphront-pager-view-css' => 'dd49ed34',
-    'aphront-panel-view-css' => 'dd49ed34',
-    'aphront-side-nav-view-css' => 'dd49ed34',
-    'aphront-table-view-css' => 'dd49ed34',
-    'aphront-tokenizer-control-css' => 'dd49ed34',
-    'aphront-tooltip-css' => 'dd49ed34',
-    'aphront-typeahead-control-css' => 'dd49ed34',
+    'aphront-headsup-view-css' => 'e3da34c9',
+    'aphront-list-filter-view-css' => 'e3da34c9',
+    'aphront-pager-view-css' => 'e3da34c9',
+    'aphront-panel-view-css' => 'e3da34c9',
+    'aphront-side-nav-view-css' => 'e3da34c9',
+    'aphront-table-view-css' => 'e3da34c9',
+    'aphront-tokenizer-control-css' => 'e3da34c9',
+    'aphront-tooltip-css' => 'e3da34c9',
+    'aphront-typeahead-control-css' => 'e3da34c9',
    'differential-changeset-view-css' => '19ebcc79',
    'differential-core-view-css' => '19ebcc79',
    'differential-inline-comment-editor' => '26972e06',
@ -3151,15 +3160,15 @@ celerity_register_resource_map(array(
    'javelin-workflow' => '971b021e',
    'maniphest-task-summary-css' => '7839ae2d',
    'maniphest-transaction-detail-css' => '7839ae2d',
-    'phabricator-app-buttons-css' => 'dd49ed34',
+    'phabricator-app-buttons-css' => 'e3da34c9',
    'phabricator-content-source-view-css' => '19ebcc79',
-    'phabricator-core-buttons-css' => 'dd49ed34',
-    'phabricator-core-css' => 'dd49ed34',
-    'phabricator-directory-css' => 'dd49ed34',
+    'phabricator-core-buttons-css' => 'e3da34c9',
+    'phabricator-core-css' => 'e3da34c9',
+    'phabricator-directory-css' => 'e3da34c9',
    'phabricator-drag-and-drop-file-upload' => '26972e06',
    'phabricator-dropdown-menu' => '971b021e',
-    'phabricator-flag-css' => 'dd49ed34',
-    'phabricator-jump-nav' => 'dd49ed34',
+    'phabricator-flag-css' => 'e3da34c9',
+    'phabricator-jump-nav' => 'e3da34c9',
    'phabricator-keyboard-shortcut' => '971b021e',
    'phabricator-keyboard-shortcut-manager' => '971b021e',
    'phabricator-menu-item' => '971b021e',
@ -3167,11 +3176,11 @@ celerity_register_resource_map(array(
    'phabricator-paste-file-upload' => '971b021e',
    'phabricator-prefab' => '971b021e',
    'phabricator-project-tag-css' => '7839ae2d',
-    'phabricator-remarkup-css' => 'dd49ed34',
+    'phabricator-remarkup-css' => 'e3da34c9',
    'phabricator-shaped-request' => '26972e06',
-    'phabricator-standard-page-view' => 'dd49ed34',
+    'phabricator-standard-page-view' => 'e3da34c9',
    'phabricator-tooltip' => '971b021e',
-    'phabricator-transaction-view-css' => 'dd49ed34',
-    'syntax-highlighting-css' => 'dd49ed34',
+    'phabricator-transaction-view-css' => 'e3da34c9',
+    'syntax-highlighting-css' => 'e3da34c9',
  ),
 ));
--- a/src/applications/paste/controller/PhabricatorPasteListController.php
+++ b/src/applications/paste/controller/PhabricatorPasteListController.php
@ -238,6 +238,7 @@ final class PhabricatorPasteListController extends PhabricatorPasteController {
    $new_paste = $this->getPaste();

    $form = new AphrontFormView();
+    $form->setFlexible(true);

    $available_languages = PhabricatorEnv::getEnvConfig(
      'pygments.dropdown-choices');
@ -281,12 +282,10 @@ final class PhabricatorPasteListController extends PhabricatorPasteController {
          ->addCancelButton('/paste/')
          ->setValue('Create Paste'));

-    $create_panel = new AphrontPanelView();
-    $create_panel->setWidth(AphrontPanelView::WIDTH_FULL);
-    $create_panel->setHeader('Create a Paste');
-    $create_panel->appendChild($form);
-
-    return $create_panel;
+    return array(
+      id(new PhabricatorHeaderView())->setHeader('Create Paste'),
+      $form,
+    );
  }

  private function buildPasteList(array $pastes) {
--- a/src/view/form/AphrontFormView.php
+++ b/src/view/form/AphrontFormView.php
@ -26,6 +26,12 @@ final class AphrontFormView extends AphrontView {
  private $user;
  private $workflow;
  private $id;
+  private $flexible;
+
+  public function setFlexible($flexible) {
+    $this->flexible = $flexible;
+    return $this;
+  }

  public function setID($id) {
    $this->id = $id;
@ -63,13 +69,22 @@ final class AphrontFormView extends AphrontView {
  }

  public function render() {
+    if ($this->flexible) {
+      require_celerity_resource('phabricator-form-view-css');
+    }
    require_celerity_resource('aphront-form-view-css');

    Javelin::initBehavior('aphront-form-disable-on-submit');

-    $layout = id(new AphrontFormLayoutView())
-      ->setBackgroundShading(true)
-      ->setPadded(true)
+    $layout = new AphrontFormLayoutView();
+
+    if (!$this->flexible) {
+      $layout
+        ->setBackgroundShading(true)
+        ->setPadded(true);
+    }
+
+    $layout
      ->appendChild($this->renderDataInputs())
      ->appendChild($this->renderChildren());

@ -80,6 +95,7 @@ final class AphrontFormView extends AphrontView {
    return phabricator_render_form(
      $this->user,
      array(
+        'class'   => $this->flexible ? 'phabricator-form-view' : null,
        'action'  => $this->action,
        'method'  => $this->method,
        'enctype' => $this->encType,
--- a/src/view/form/control/AphrontFormControl.php
+++ b/src/view/form/control/AphrontFormControl.php
@ -1,7 +1,7 @@
 <?php

 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@ -127,7 +127,6 @@ abstract class AphrontFormControl extends AphrontView {
      $label =
        '<label class="aphront-form-label">'.
          phutil_escape_html($this->getLabel()).
-          ':'.
        '</label>';
    } else {
      $label = null;
@ -142,14 +141,16 @@ abstract class AphrontFormControl extends AphrontView {
    if (strlen($this->getError())) {
      $error = $this->getError();
      if ($error === true) {
-        $error = '*';
+        $error =
+          '<div class="aphront-form-error aphront-form-required">'.
+            'Required'.
+          '</div>';
      } else {
-        $error = "\xC2\xAB ".$error;
+        $error =
+          '<div class="aphront-form-error">'.
+            phutil_escape_html($error).
+          '</div>';
      }
-      $error =
-        '<div class="aphront-form-error">'.
-          phutil_escape_html($error).
-        '</div>';
    } else {
      $error = null;
    }
@ -170,10 +171,12 @@ abstract class AphrontFormControl extends AphrontView {
        'id' => $this->controlID,
        'style' => $this->controlStyle,
      ),
-      $error.
      $label.
+      $error.
      $input.
      $caption.
+
+      // TODO: Remove this once the redesign finishes up.
      '<div style="clear: both;"></div>');
  }
 }
--- a/webroot/rsrc/css/aphront/form-view.css
+++ b/webroot/rsrc/css/aphront/form-view.css
@ -40,6 +40,12 @@
  padding-top: 4px;
 }

+.aphront-form-required {
+  font-weight: normal;
+  color: #888888;
+  font-size: 11px;
+}
+
 .aphront-form-input input,
 .aphront-form-input textarea {
  font-size: 12px;
@ -78,7 +84,6 @@
  font-size: 11px;
  color: #444444;
  text-align: right;
-  clear: both;
  margin-right: 25%;
  margin-left: 20%;
 }
--- a/webroot/rsrc/css/layout/phabricator-form-view.css
+++ b/webroot/rsrc/css/layout/phabricator-form-view.css
@ -0,0 +1,56 @@
+/**
+ * @provides phabricator-form-view-css
+ */
+
+.phabricator-form-view {
+  padding: .75em;
+  overflow: hidden;
+
+  background: #f9f9f9;
+
+  border-width: 1px 0;
+  border-style: solid;
+  border-color: #dbdbdb;
+}
+
+
+.device-tablet .phabricator-form-view .aphront-form-control .aphront-form-label,
+.device-phone .phabricator-form-view .aphront-form-control .aphront-form-label {
+  display: block;
+  float: none;
+  text-align: left;
+  margin: 0;
+  padding: 0;
+}
+
+.device-tablet .phabricator-form-view .aphront-form-control
+  .aphront-form-caption,
+.device-phone .phabricator-form-view .aphront-form-control
+  .aphront-form-caption {
+  display: block;
+  float: none;
+  text-align: left;
+  margin: 0;
+  padding: 0;
+}
+
+.device-tablet .phabricator-form-view .aphront-form-control
+  .aphront-form-input,
+.device-phone .phabricator-form-view .aphront-form-control
+  .aphront-form-input {
+  float: none;
+  display: block;
+  text-align: left;
+  margin: .25em 0 0 0;
+  width: 99%;
+  padding: 0;
+}
+
+.device-tablet .phabricator-form-view .aphront-form-control .aphront-form-error,
+.device-phone .phabricator-form-view .aphront-form-control .aphront-form-error {
+  float: none;
+  text-align: left;
+  display: block;
+  margin: 0;
+  padding: 0;
+}