From 3301b5850a5dffacb0b308efad5e010f73038fd5 Mon Sep 17 00:00:00 2001
From: Maxim Colls <collsmaxim@gmail.com>
Date: Sat, 17 Nov 2018 18:48:10 +0100
Subject: [PATCH] Improved UX in the accordion steps in the checkout page

---
 .../checkout/accordion_controller.js.coffee   | 25 ++++++++-----------
 .../checkout/billing_controller.js.coffee     |  4 +--
 .../checkout/details_controller.js.coffee     |  6 ++---
 .../darkswarm/mixins/fieldset_mixin.js.coffee |  4 +--
 app/views/checkout/edit.html.haml             |  2 +-
 5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee
index e85f9e5cee..d61ff6ceb7 100644
--- a/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/checkout/accordion_controller.js.coffee
@@ -1,25 +1,22 @@
 Darkswarm.controller "AccordionCtrl", ($scope, localStorageService, $timeout, $document, CurrentHub) ->
-  key = "accordion_#{$scope.order.id}#{CurrentHub.hub.id}#{$scope.order.user_id}"
-  value = if localStorageService.get(key) then {} else { details: true, billing: false, shipping: false, payment: false }
-  localStorageService.bind $scope, "accordion", value, key
   $scope.accordionSections = ["details", "billing", "shipping", "payment"]
-  # Scrolling is confused by our position:fixed top bar - add an offset to scroll
-  # to the correct location, plus 5px buffer
-  offset_height = $("nav.top-bar").height() + 5
+  $scope.accordion = { details: true, billing: true, shipping: true, payment: true }
 
-  $scope.show = (section)->
+  $scope.show = (section) ->
     $scope.accordion[section] = true
-    # If we call scrollTo() directly after show(), when one of the accordions above the
-    # scroll location is closed by show(), scrollTo() will scroll to the old location of
-    # the element. Putting this in a 50 ms timeout is enough delay for the DOM to
-    # have updated.
-    $timeout ->
-      $document.scrollTo($("##{section}"), offset_height, 500)
-    , 50
+
+  $scope.scrollTo = (section) ->
+    # Scrolling is confused by our position:fixed top bar - add an offset to scroll
+    # to the correct location, plus 5px buffer
+    offset_height = $("nav.top-bar").height() + 5
+    $document.scrollTo($("##{section}"), offset_height, 400)
 
   $scope.$on 'purchaseFormInvalid', (event, form) ->
     # Scroll to first invalid section
     for section in $scope.accordionSections
       if not form[section].$valid
         $scope.show section
+        $timeout ->
+          $scope.scrollTo(section)
+        , 50
         break
diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee
index b8dec8b190..b7246fddd9 100644
--- a/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/checkout/billing_controller.js.coffee
@@ -5,7 +5,7 @@ Darkswarm.controller "BillingCtrl", ($scope, $timeout) ->
 
   $scope.summary = ->
     [$scope.order.bill_address.address1,
-    $scope.order.bill_address.city, 
+    $scope.order.bill_address.city,
     $scope.order.bill_address.zipcode]
 
-  $timeout $scope.onTimeout 
+  $timeout $scope.onTimeout
diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee
index e8a6b1ff23..30c243ed7c 100644
--- a/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee
+++ b/app/assets/javascripts/darkswarm/controllers/checkout/details_controller.js.coffee
@@ -13,11 +13,11 @@ Darkswarm.controller "DetailsCtrl", ($scope, $timeout, $http, CurrentUser, Authe
 
   $scope.summary = ->
     [$scope.fullName(),
-    $scope.order.email, 
+    $scope.order.email,
     $scope.order.bill_address.phone]
 
   $scope.fullName = ->
-    [$scope.order.bill_address.firstname ? null, 
+    [$scope.order.bill_address.firstname ? null,
     $scope.order.bill_address.lastname ? null].join(" ").trim()
 
-  $timeout $scope.onTimeout 
+  $timeout $scope.onTimeout
diff --git a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee
index 739d2de9bd..d8bb1b262e 100644
--- a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee
+++ b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee
@@ -2,11 +2,11 @@ window.FieldsetMixin = ($scope)->
   $scope.next = (event = false)->
     event.preventDefault() if event
     return unless $scope.nextPanel
+    $scope.accordion[$scope.name] = false
     $scope.show $scope.nextPanel
 
   $scope.onTimeout = ->
-    if $scope[$scope.name].$valid
-      $scope.next()
+    $scope.accordion[$scope.name] = !$scope[$scope.name].$valid
 
   $scope.valid = ->
     $scope.form().$valid
diff --git a/app/views/checkout/edit.html.haml b/app/views/checkout/edit.html.haml
index 75c9eff08f..ab5253538b 100644
--- a/app/views/checkout/edit.html.haml
+++ b/app/views/checkout/edit.html.haml
@@ -16,7 +16,7 @@
 
   = render partial: "shopping_shared/details"
 
-  %accordion{"close-others" => "true"}
+  %accordion{"close-others" => "false"}
     %checkout.row{"ng-controller" => "CheckoutCtrl"}
       .small-12.medium-8.large-9.columns
         - unless spree_current_user