Merge pull request #2416 from nebulab/improve-frontend-forms

Improve frontend checkout forms html
solidusio · May 3, 2018 · a02685c · a02685c
2 parents 679674b + a4bc749
commit a02685c
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 64 deletions.
diff --git a/frontend/app/assets/javascripts/spree/frontend/checkout/address.js b/frontend/app/assets/javascripts/spree/frontend/checkout/address.js
@@ -43,7 +43,6 @@ Spree.ready(function($) {
       var statePara = $("#" + region + "state");
       var stateSelect = statePara.find("select");
       var stateInput = statePara.find("input");
-      var stateSpanRequired = statePara.find('[id$="state-required"]');
       if (states.length > 0) {
         var selected = parseInt(stateSelect.val());
         stateSelect.html("");
@@ -68,21 +67,21 @@ Spree.ready(function($) {
         statePara.show();
         if (statesRequired) {
           stateSelect.addClass("required");
-          stateSpanRequired.show();
+          statePara.addClass("field-required");
         } else {
           stateSelect.removeClass("required");
-          stateSpanRequired.hide();
+          statePara.removeClass("field-required");
         }
         stateInput.removeClass("required");
       } else {
         stateSelect.hide().prop("disabled", true);
         stateInput.show();
         if (statesRequired) {
-          stateSpanRequired.show();
+          statePara.addClass("field-required");
           stateInput.addClass("required");
         } else {
           stateInput.val("");
-          stateSpanRequired.hide();
+          statePara.removeClass("field-required");
           stateInput.removeClass("required");
         }
         statePara.toggle(!!statesRequired);

diff --git a/frontend/app/assets/stylesheets/spree/frontend/screen.css.scss b/frontend/app/assets/stylesheets/spree/frontend/screen.css.scss
@@ -143,10 +143,27 @@ label.error {
   margin-top: 3px;
 }
 
-span.required {
-  color: $c_red;
-  font-weight: bold;
-  font-size: 1.2em;
+.field {
+  padding: 10px 0;
+
+  label {
+    display: inline-block;
+    margin-bottom: 5px;
+  }
+
+  input,
+  select {
+    display: block;
+  }
+
+  &-required label:after {
+    content: "*";
+    display: inline-block;
+    color: $c_red;
+    font-weight: bold;
+    font-size: 1.2em;
+    margin-left: 3px;
+  }
 }
 
 fieldset {
@@ -645,6 +662,10 @@ ul#products {
 /*--------------------------------------*/
 /* Checkout
 /*--------------------------------------*/
+.checkout_form_wrapper {
+  text-align: left;
+}
+
 .out-of-stock {
   background: #df0000;
   color: white;
@@ -881,7 +902,7 @@ ul#products {
   .shipping-methods {
     list-style: none;
     margin: 0;
-    padding: none;
+    padding: 10px 0;
 
     .shipping-method {
       display: inline-block;

diff --git a/frontend/app/views/spree/address/_form.html.erb b/frontend/app/views/spree/address/_form.html.erb
@@ -1,52 +1,52 @@
 <% address_id = address_type.chars.first %>
 <div class="inner" data-hook=<%="#{address_type}_inner" %>>
-  <p class="field" id=<%="#{address_id}firstname" %>>
-    <%= form.label :firstname, t('spree.first_name') %><span class="required">*</span><br />
+  <div class="field field-required" id=<%="#{address_id}firstname" %>>
+    <%= form.label :firstname, t('spree.first_name') %>
     <%= form.text_field :firstname, class: 'required', autocomplete: address_type + ' given-name', required: true, autofocus: true %>
-  </p>
+  </div>
 
-  <p class="field" id=<%="#{address_id}lastname" %>>
-    <%= form.label :lastname, t('spree.last_name') %><br />
+  <div class="field" id=<%="#{address_id}lastname" %>>
+    <%= form.label :lastname, t('spree.last_name') %>
     <%= form.text_field :lastname, autocomplete: address_type + ' family-name' %>
-  </p>
+  </div>
 
   <% if Spree::Config[:company] %>
-    <p class="field" id=<%="#{address_id}company" %>>
-      <%= form.label :company, t('spree.company') %><br />
+    <div class="field" id=<%="#{address_id}company" %>>
+      <%= form.label :company, t('spree.company') %>
       <%= form.text_field :company, autocomplete: address_type + ' organization' %>
-    </p>
+    </div>
   <% end %>
 
-  <p class="field" id=<%="#{address_id}address1" %>>
-    <%= form.label :address1, t('spree.street_address') %><span class="required">*</span><br />
+  <div class="field field-required" id=<%="#{address_id}address1" %>>
+    <%= form.label :address1, t('spree.street_address') %>
     <%= form.text_field :address1, class: 'required', autocomplete: address_type + ' address-line1',  required: true %>
-  </p>
+  </div>
 
-  <p class="field" id=<%="#{address_id}address2" %>>
-    <%= form.label :address2, I18n.t('spree.street_address_2') %><br />
+  <div class="field" id=<%="#{address_id}address2" %>>
+    <%= form.label :address2, I18n.t('spree.street_address_2') %>
     <%= form.text_field :address2, autocomplete: address_type + ' address-line2' %>
-  </p>
+  </div>
 
-  <p class="field" id=<%="#{address_id}city" %>>
-    <%= form.label :city, t('spree.city') %><span class="required">*</span><br />
+  <div class="field field-required" id=<%="#{address_id}city" %>>
+    <%= form.label :city, t('spree.city') %>
     <%= form.text_field :city, class: 'required', autocomplete: address_type + ' address-level2',  required: true %>
-  </p>
+  </div>
 
-  <p class="field" id=<%="#{address_id}country" %>>
-    <%= form.label :country_id, t('spree.country') %><span class="required">*</span><br />
+  <div class="field field-required" id=<%="#{address_id}country" %>>
+    <%= form.label :country_id, t('spree.country') %>
     <span id=<%="#{address_id}country-selection" %>>
       <%= form.collection_select :country_id, available_countries, :id, :name, {},
           class: 'required',
           autocomplete: address_type + ' country-name',
           required: true
         %>
     </span>
-  </p>
+  </div>
 
   <% if Spree::Config[:address_requires_state] %>
-    <p class="field" id=<%="#{address_id}state" %>>
+    <div class="field field-required" id=<%="#{address_id}state" %>>
       <% have_states = !address.country.states.empty? %>
-      <%= form.label :state, t('spree.state') %><span class='required' id=<%="#{address_id}state-required"%>>*</span><br/>
+      <%= form.label :state, t('spree.state') %>
 
       <span class="js-address-fields" style="display: none;">
         <%=
@@ -73,24 +73,24 @@
       <noscript>
         <%= form.text_field :state_name, class: 'required', autocomplete: address_type + ' address-level1',  required: true %>
       </noscript>
-    </p>
+    </div>
   <% end %>
 
-  <p class="field" id=<%="#{address_id}zipcode" %>>
-    <%= form.label :zipcode, t('spree.zip') %><% if address.require_zipcode? %><span class="required">*</span><% end %><br />
+  <div class="field <%= 'field-required' if address.require_zipcode? %>" id=<%="#{address_id}zipcode" %>>
+    <%= form.label :zipcode, t('spree.zip') %>
     <%= form.text_field :zipcode, class: "#{'required' if address.require_zipcode?}", autocomplete: address_type + ' postal-code',  required: true %>
-  </p>
+  </div>
 
-  <p class="field" id=<%="#{address_id}phone" %>>
-    <%= form.label :phone, t('spree.phone') %><% if address.require_phone? %><span class="required">*</span><% end %><br />
+  <div class="field <%= 'field-required' if address.require_phone? %>" id=<%="#{address_id}phone" %>>
+    <%= form.label :phone, t('spree.phone') %>
     <% phone_hash = address.require_phone? ? { class: 'required', required: true } : {} %>
     <%= form.phone_field :phone, phone_hash.merge({ autocomplete: address_type + ' home tel' }) %>
-  </p>
+  </div>
 
   <% if Spree::Config[:alternative_shipping_phone] %>
-    <p class="field" id=<%="#{address_id}altphone" %>>
-      <%= form.label :alternative_phone, t('spree.alternative_phone') %><br />
+    <div class="field" id=<%="#{address_id}altphone" %>>
+      <%= form.label :alternative_phone, t('spree.alternative_phone') %>
       <%= form.phone_field :alternative_phone, autocomplete: address_type + ' tel'%>
-    </p>
+    </div>
   <% end %>
 </div>
diff --git a/frontend/app/views/spree/checkout/_address.html.erb b/frontend/app/views/spree/checkout/_address.html.erb
@@ -11,10 +11,10 @@
   <fieldset id="shipping" data-hook>
     <%= form.fields_for :ship_address do |ship_form| %>
       <legend align="center"><%= t('spree.shipping_address') %></legend>
-      <p class="field checkbox" data-hook="use_billing">
+      <div class="checkbox" data-hook="use_billing">
         <%= check_box_tag 'order[use_billing]', '1', @order.shipping_eq_billing_address? %>
         <%= label_tag :order_use_billing, t('spree.use_billing_address'), id: 'use_billing' %>
-      </p>
+      </div>
       <%= render partial: 'spree/address/form', locals: { form: ship_form, address_type: 'shipping', address: @order.ship_address } %>
     <% end %>
   </fieldset>

diff --git a/frontend/app/views/spree/checkout/_delivery.html.erb b/frontend/app/views/spree/checkout/_delivery.html.erb
@@ -38,7 +38,7 @@
           </table>
 
           <h5 class="stock-shipping-method-title"><%= t('spree.shipping_method') %></h5>
-          <ul class="field radios shipping-methods">
+          <ul class="radios shipping-methods">
             <% ship_form.object.shipping_rates.each do |rate| %>
               <li class="shipping-method">
                 <label>

diff --git a/frontend/app/views/spree/checkout/_payment.html.erb b/frontend/app/views/spree/checkout/_payment.html.erb
@@ -15,7 +15,7 @@
       </div>
 
       <div id="existing_cards">
-        <p class="field" data-hook="existing_cards">
+        <div class="field" data-hook="existing_cards">
           <table class="existing-credit-card-list">
             <tbody>
               <% @wallet_payment_sources.each do |wallet_payment_source| %>
@@ -31,7 +31,7 @@
               <% end %>
             </tbody>
           </table>
-        </p>
+        </div>
       </div>
     <% end %>
 
@@ -57,7 +57,7 @@
     </ul>
     <br style="clear:both;" />
     <p class='field' data-hook='coupon_code'>
-      <%= form.label :coupon_code %><br />
+      <%= form.label :coupon_code %>
       <%= form.text_field :coupon_code %>
       <button type="button" class="button" id="coupon-code-apply-button">
         <%= t('spree.apply_code') %>

diff --git a/frontend/app/views/spree/checkout/edit.html.erb b/frontend/app/views/spree/checkout/edit.html.erb
@@ -10,10 +10,10 @@
     <div class="columns <%= if @order.state != 'confirm' then 'alpha twelve' else 'alpha omega sixteen' end %>" data-hook="checkout_form_wrapper">
       <%= form_for @order, url: update_checkout_path(@order.state), html: { id: "checkout_form_#{@order.state}" } do |form| %>
         <% if @order.state == 'address' || !@order.email? %>
-          <p class="field" style='clear: both'>
+          <div class="field field-required" style='clear: both'>
             <%= form.label :email %><br />
             <%= form.email_field :email, required: true %>
-          </p>
+          </div>
         <% end %>
         <%= render @order.state, form: form %>
       <% end %>

diff --git a/frontend/app/views/spree/checkout/payment/_gateway.html.erb b/frontend/app/views/spree/checkout/payment/_gateway.html.erb
@@ -1,32 +1,31 @@
 <%= image_tag 'credit_cards/credit_card.gif', id: 'credit-card-image' %>
 <% param_prefix = "payment_source[#{payment_method.id}]" %>
 
-<p class="field">
-  <%= label_tag "name_on_card_#{payment_method.id}", t('spree.name_on_card') %><span class="required">*</span><br />
+<div class="field field-required">
+  <%= label_tag "name_on_card_#{payment_method.id}", t('spree.name_on_card') %>
   <%= text_field_tag "#{param_prefix}[name]", "#{@order.billing_firstname} #{@order.billing_lastname}", { id: "name_on_card_#{payment_method.id}", autocomplete: "cc-name" } %>
-</p>
+</div>
 
-<p class="field" data-hook="card_number">
-  <%= label_tag "card_number", t('spree.card_number') %><span class="required">*</span><br />
+<div class="field field-required" data-hook="card_number">
+  <%= label_tag "card_number", t('spree.card_number') %>
   <%= text_field_tag "#{param_prefix}[number]", '', {id: 'card_number', class: 'required cardNumber', size: 19, maxlength: 19, autocomplete: "cc-number", type: "tel" } %>
-  &nbsp;
   <span id="card_type" style="display:none;">
     ( <span id="looks_like" ><%= t('spree.card_type_is') %> <span id="type"></span></span>
       <span id="unrecognized"><%= t('spree.unrecognized_card_type') %></span>
     )
   </span>
-</p>
+</div>
 
-<p class="field" data-hook="card_expiration">
-  <%= label_tag "card_expiry", t('spree.expiration') %><span class="required">*</span><br />
+<div class="field field-required" data-hook="card_expiration">
+  <%= label_tag "card_expiry", t('spree.expiration') %>
   <%= text_field_tag "#{param_prefix}[expiry]", '', id: 'card_expiry', class: "required cardExpiry", placeholder: "MM / YY", type: "tel" %>
-</p>
+</div>
 
-<p class="field" data-hook="card_code">
-  <%= label_tag "card_code", t('spree.card_code') %><span class="required">*</span><br />
+<div class="field field-required" data-hook="card_code">
+  <%= label_tag "card_code", t('spree.card_code') %>
   <%= text_field_tag "#{param_prefix}[verification_value]", '', {id: 'card_code', class: 'required cardCode', size: 5, type: "tel", autocomplete: "off" } %>
   <%= link_to "(#{t('spree.what_is_this')})", spree.cvv_path, target: '_blank', "data-hook" => "cvv_link", id: "cvv_link" %>
-</p>
+</div>
 
 <% if @order.bill_address %>
   <%= fields_for "#{param_prefix}[address_attributes]", @order.bill_address do |f| %>