Form / validation improvements

* Refactor emailform.php error/validation logic: * Now highlights optional inputs, rather than required inputs, for consistency with the main #writeForm. * Adds a `required` attribute to required inputs, for client-side form validation. * Wraps related radio inputs in a `<fieldset>`, with the group title in a `<legend>`. * "Your name" / "Your email" input labels on emailform.php, for consistency with the main #writeForm. * Styling for error lists at the top of a submitted form. * Styling for fieldsets, legends, and errored inputs. * Remove `_overrides.scss` file, since it only had one rule in it.
mysociety · Mar 23, 2021 · b29c065 · b29c065
1 parent 67d3b6c
commit b29c065
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 61 deletions.
diff --git a/phplib/emailform.php b/phplib/emailform.php
@@ -22,14 +22,14 @@
     array ('label' => 'Other', 'value' => 'other')
 );
 $emailformfields = array (
-    array ('label'        => 'Name',
+    array ('label'        => 'Your name',
            'inputname'    => 'name',
            'inputtype'    => 'text',
            'size'         => '30',
            'spamcheck'    => "1",
            'errormessage' => 'Please enter your name',
            'required'     => 1),
-    array ('label'       => 'Email Address',
+    array ('label'       => 'Your email',
            'inputname'   => 'emailaddy',
            'inputtype'   => 'text',
            'size'        => '30',
@@ -94,19 +94,23 @@ function fyr_display_emailform () {
 
 function render_formfield ($defs, $messages) {
   $input = '';
-  $value='';
+  $value = '';
   if (isset($defs['value']))
       $value = $defs['value'];
   if (isset($_POST[$defs['inputname']]))
       $value = $_POST[$defs['inputname']];
+
   $htmlvalue = htmlentities($value, ENT_QUOTES, 'UTF-8');
+  $class = isset($messages[$defs['inputname']]) ? 'error' : '';
+  $required = ( $defs['required'] ?? 0 ) ? ' required' : '';
+  $label_suffix = '';
 
   if ($defs['inputtype'] == 'text') {
-      $input = '<input type="text" name="' . $defs['inputname'] . '" id="' . $defs['inputname'] . '" size="' . $defs['size'] . '" value="' . $htmlvalue . '">';
+      $input = '<input type="text" name="' . $defs['inputname'] . '" id="' . $defs['inputname'] . '" size="' . $defs['size'] . '" value="' . $htmlvalue . '" class="' . $class .'"' . $required . '>';
   }
   if ($defs['inputtype'] == 'textarea') {
       $sizes = explode(",", $defs['size']);
-      $input = '<textarea name="' . $defs['inputname'] . '" id="' . $defs['inputname'] . '" rows="' . $sizes[0] . '" cols="' . $sizes[1] . '">' . $htmlvalue . '</textarea>';
+      $input = '<textarea name="' . $defs['inputname'] . '" id="' . $defs['inputname'] . '" rows="' . $sizes[0] . '" cols="' . $sizes[1] . '" class="' . $class .'"' . $required . '>' . $htmlvalue . '</textarea>';
   }
   if ($defs['inputtype'] == 'submit') {
       $input = '<input name="' . $defs['inputname'] . '" id="' . $defs['inputname'] . '" type="submit" value="' . $defs['value'] . '" class="button success">';
@@ -116,37 +120,35 @@ function render_formfield ($defs, $messages) {
       foreach ($radiovalues as $radvalue) {
           $element_id = $defs['inputname'] . '_' . $radvalue['value'];
           $checked = ($value == $radvalue['value']) ? ' checked' : '';
-          $input .= '<input name="' . $defs['inputname'] . '" id="' . $element_id . '" type="radio" value="' . $radvalue['value'] . '" ' . $checked . '> ';
-          $input .= '<label class="inline-label" for="' . $element_id . '">' . $radvalue['label'] . '</label><br>';
+          $input .= '<p>';
+          $input .= '<input name="' . $defs['inputname'] . '" id="' . $element_id . '" type="radio" value="' . $radvalue['value'] . '"' . $checked . $required . '> ';
+          $input .= '<label class="inline-label ' . $class . '" for="' . $element_id . '">' . $radvalue['label'] . '</label>';
+          $input .= '</p>';
       }
   }
 
+  if ($defs['inputtype'] != 'submit' && ( !( $defs['required'] ?? 0 ) )) {
+      $label_suffix = ' <span class="optional-text">optional</span>';
+  }
+
   if (isset($defs['label'])){
-    $label = $defs['label'];
-    if (isset($defs['required']) && $defs['required']) {
-        $label .= ' (required)';
-    }
-    if ($label) $label .= ':';
     $out = '<label for="' . $defs['inputname'] . '"';
     if (isset($messages[$defs['inputname']]))
-        $out .= ' class="repwarning"';
-    $out .= '>' . $label . '</label>' . $input ;
-  }else{
-    $intro_text = $defs['intro_text'];
-    if (isset($defs['required']) && $defs['required']) {
-        $intro_text .= ' (required)';
-    }
-    if ($intro_text) $intro_text .= ':';
-    $out = '<span class="intro-text';
+        $out .= ' class="error"';
+    $out .= '>' . $defs['label'] . $label_suffix . '</label>' . $input ;
+    return '<p>' . $out . '</p>';
+
+  } else {
+    $out = '<legend';
     if (isset($messages[$defs['inputname']]))
-        $out .= ' repwarning';
-    $out .= '">' . $intro_text . '</span><br>' . $input ;
+        $out .= ' class="error"';
+    $out .= '>' . $defs['intro_text'] . $label_suffix . '</legend>' . $input ;
+    return '<fieldset>' . $out . '</fieldset>';
   }
-  return '<p>' . $out . '</p>';
 }
 
 function wrongcontact_display($problem, $contact_message) {
-  print '<div id ="sendmess">';
+  print '<div id="sendmess">';
   print '<div class="wrong-contact">';
   print $problem;
   print '<hr>';
@@ -157,19 +159,22 @@ function wrongcontact_display($problem, $contact_message) {
 
 function emailform_display ($messages) {
     global $emailformfields;
+
     if ($messages && isset($messages['messagesent'])){
       print '<p class="alertsuccess">' . $messages['messagesent']  . '</p>';
       return;
     }
-    print '<div id ="sendmess">';
-    if ($messages) {
 
-      print '<ul class="repwarning">';
+    print '<div id="sendmess">';
+
+    if ($messages) {
+      print '<ul class="errors">';
       foreach ($messages as $inp => $mess) {
           print '<li>' . $mess;
       }
       print '</ul>';
-
+    } else {
+        print '<p><strong class="text-warning">This message will not go to your MP.</strong></p>';
     }
     print '<form action="about-contactresponse" accept-charset="utf8" method="post">';
     print '<input name="action" type="hidden" value="testmess">';

diff --git a/templates/website/about-contact.html b/templates/website/about-contact.html
@@ -129,9 +129,6 @@ <h3>
                                 website, or have a question about how it works,
                                 you should contact us using the form below.
                             </p>
-                            <p>
-                                <strong class="red">This message will not go to your MP.</strong>
-                            </p>
                             <?php fyr_display_emailform($values); ?>
                         </div>
                     </div>

diff --git a/web/static/css/wtt.css b/web/static/css/wtt.css
diff --git a/web/static/sass/_contact-options.scss b/web/static/sass/_contact-options.scss
@@ -35,10 +35,10 @@ $contact-option-padding: 20px;
         min-width: 100%;
         max-width: 100%;
     }
+}
 
-    .red {
-        color: $alert-color;
-    }
+.text-warning {
+    color: $alert-color;
 }
 
 .contact-option {

diff --git a/web/static/sass/_overrides.scss b/web/static/sass/_overrides.scss
diff --git a/web/static/sass/_settings.scss b/web/static/sass/_settings.scss
@@ -302,17 +302,17 @@ $input-box-shadow: none;
 
 // We use these to style the fieldset border and spacing.
 
-// $fieldset-border-style: solid;
+$fieldset-border-style: none; // default: solid
 // $fieldset-border-width: 1px;
 // $fieldset-border-color: #ddd;
-// $fieldset-padding: emCalc(20);
-// $fieldset-margin: emCalc(18, 0);
+$fieldset-padding: 0; // default: emCalc(20)
+$fieldset-margin: 0; //default: emCalc(18, 0)
 
 // We use these to style the legends when you use them
 
-// $legend-bg: #fff;
-// $legend-font-weight: bold;
-// $legend-padding: emCalc(0, 3);
+$legend-bg: transparent; // default: #fff
+$legend-font-weight: 600; // default: bold
+$legend-padding: 0; // default: emCalc(0, 3)
 
 // We use these to style the prefix and postfix input elements
 

diff --git a/web/static/sass/wtt.scss b/web/static/sass/wtt.scss
@@ -159,6 +159,59 @@ textarea {
     }
 }
 
+input.error,
+textarea.error {
+    background-color: #fff; // override Foundation default
+}
+
+// Vertical space between label text and input, when nested.
+label input {
+    margin-top: 0.2em;
+}
+
+label .optional-text,
+legend .optional-text {
+    font-size: emCalc(12);
+    opacity: 0.8;
+    float: right;
+    font-family: $header-font-family;
+}
+
+fieldset {
+    legend {
+        display: block;
+        width: 100%;
+        margin-bottom: $paragraph-margin-bottom;
+    }
+
+    p {
+        margin-bottom: 0;
+    }
+}
+
+fieldset.error legend,
+legend.error {
+    color: $alert-color;
+}
+
+.inline-label {
+    display: inline;
+    margin: 0 0.3em;
+}
+
+ul.errors {
+    color: $alert-color;
+    margin: 1.5em 0;
+
+    li {
+        margin-bottom: 0;
+    }
+
+    li + li {
+        margin-top: 0.5em;
+    }
+}
+
 /*
  * Tables
  */
@@ -951,9 +1004,7 @@ form#writeForm {
     }
 
     fieldset {
-        border: none;
         legend {
-            background: none;
             font-family: $header-font-family;
             font-size: emCalc(20);
             font-weight: 600;
@@ -987,19 +1038,6 @@ form#writeForm {
       }
     }
 
-    label {
-        input {
-            margin-top: 0.2em;
-        }
-    }
-
-    .optional-text {
-        font-size: emCalc(12);
-        color: #666;
-        float: right;
-        font-family: $header-font-family;
-    }
-
     .error-message {
         color: $alert-color;
         font-family: $header-font-family;
@@ -1308,7 +1346,6 @@ h2 {
 }
 
 @import "contact-options";
-@import "overrides";
 
 /* 
  * WTT political knowledge survey