Shipping info update by gmemstr · Pull Request #2 · AcroMedia/orange_ecom_starter

gmemstr · 2018-07-27T17:49:08Z

Small changes that allow us to remove the Recalculate shipping button completely in favour of displaying them instead when the postal code is complete. Also allows us to update the shipping method when selected instead of when we submit the form and go to the next page.

…hod select

…is completed

travis-bradbury · 2018-07-31T23:32:31Z

orange_ecom_starter.theme

+ */
+function orange_ecom_starter_commerce_checkout_pane_info_alter(&$checkout_panes) {
+  if($checkout_panes['shipping_information']) { return false; }
+}


The file doesn't end with a newline. That's required by the code standard. This is easier if you set PhpStorm to always add it automatically (in Settings > Editor > General, select "Ensure line feed at file end on Save").

travis-bradbury · 2018-07-31T23:34:56Z

orange_ecom_starter.theme

+ * Implements hook_commerce_checkout_pane_info_alter(&$checkout_panes).
+ */
+function orange_ecom_starter_commerce_checkout_pane_info_alter(&$checkout_panes) {
+  if($checkout_panes['shipping_information']) { return false; }


What's the purpose of this? I don't think it has any effect unless you change something in $checkout_panes.

There's two code standards issues:

if( should be if (.

The body of this block and the } should each be on new lines.

If this does do something, please add a comment for what it does and why we need to do it that way - I'm sure I wouldn't be the only one confused by it.

This has been removed completely since it didn't actually do anything.

travis-bradbury · 2018-07-31T23:42:59Z

js/custom/script.js

+          console.log('blur!');
+          $('input[id^=edit-shipping-information-recalculate-shipping]').mousedown();
+      });
+  }


I'm sure this hack isn't the solution we want (it may not work the same on sites that have been customized and it doesn't help other users of commerce_shipping who could use the same feature), but I'm not really sure what the proper solution is. See https://www.drupal.org/project/commerce_shipping/issues/2898118. Bojan described this as "not easy" but maybe changes to Commerce in the last few months have made things better.

It would be a good idea to get some direction from Bojan in the commerce channel of Drupal Slack (https://www.drupal.org/slack) about how to implement what we want (fixing it upstream in commerce or commerce_shipping rather than just in Drupal Orange).

…d selection on cart form

We can't seem to add ajax listeners after the fact using #after_build, so the js listener is going back in as our best bet - we might be able to avoid clicking the button though and instead trigger an event durpal- side.

travis-bradbury · 2018-08-09T21:02:07Z

orange_ecom_starter.theme

+        'class' => array('hidden'),
+      ];
+    }
+    // edit-shipping-information-shipping-profile-address-0-address-postal-code


What's this comment about? Is this something that should be removed?

That was a note to myself about where something was located and I forgot about, removed.

travis-bradbury · 2018-08-09T21:04:09Z

orange_ecom_starter.theme

+ * Ajax callback for refreshing the order shipping method, this will eventually
+ * be moved to the commerce shipping module instead.
+ */
+function ajaxRefreshSummary(array $form, FormStateInterface $form_state) {


This function name must be snake case (ajax_refresh_summary) not camel case (ajaxRefreshSummary). In the Drupal standard functions use underscores and class methods use camel case.

The name must also be prefixed with the module or theme name: orange_ecom_starter_ajax_refresh_summary instead of ajax_request_summary.

travis-bradbury · 2018-08-09T21:06:29Z

orange_ecom_starter.theme

+
+/**
+ * Ajax callback for refreshing the order shipping method, this will eventually
+ * be moved to the commerce shipping module instead.


The first line of a documentation block must be its own paragraph under 80 characters. Further text would be another paragraph.

/** * Ajax callback for refreshing the order shipping method. * * Something about how this should be accomplished by commerce_shipping and a * link to the relevant Drupal.org issue. */

travis-bradbury · 2018-08-09T21:20:47Z

js/custom/script.js

+  // Mousedown on hidden button to trigger price recalculation when postal code input is out of focus, because
+  // we can't override it in our theme hooks.
+  if($('#edit-shipping-information-recalculate-shipping').length) {
+      $('body').on('blur', 'input.postal-code', function () {


Since we're using blur, this doesn't activate until I've entered my postal code and then try to do something else. You could end up trying to put in your payment information and having your available shipping methods change, which is weird since we've moved past that, or probably more likely, have these weird issues selecting

Filling out the address, there are no shipping methods, just in-store. It looks like the only option, so if I want it shipped I'm now confused. Or, I settle for in-store pickup and go to payment information, and now the methods update.

Or you could see the methods, make a change and try to select a method, but you can't actually select it because removing focus from the postal code submitted the form instead.

Is it feasible to use the keydown event, but instead of doing the recalculate callback right away, start a short timer? When the timer expires the recalculation happens, but each event resets the timer so it doesn't make the ajax calls while you're still typing.

I think the issue with this (after some playing around) is that if you take slightly longer to input your postal code it will try to update too early, but if the delay is too long we end up with roughly the same result but with more complexity. The trick would be getting the delay right, but I'm not sure what that "magic number" would be.

…andards issues

Instead of a blue or change of the postal code, we add a listener to every input element instead, and wait until all the required fields have been filled.

gmemstr · 2018-08-17T16:25:59Z

So the new triggering method requires all the required fields to be filled in before it "clicks" the button, and activates on keyup in any input field.

Gabriel Simmer added 4 commits July 27, 2018 08:51

Basics of AJAX working for updating shipment price on radio button click

2028574

Completely remove custom ajax function for ::submitForm instead

53831ca

Cleaned up .theme and added custom JS to refresh page on shipping met…

4e494ae

…hod select

Remove recalculate button and instead trigger event when postal code …

e5c64f1

…is completed

travis-bradbury suggested changes Jul 31, 2018

View reviewed changes

Some ajax changes (ignore)

8868b68

gmemstr force-pushed the shipping-info-update branch from 9f98513 to 8868b68 Compare August 8, 2018 23:11

Gabriel Simmer added 4 commits August 8, 2018 16:12

Remove javasript event listener and added callback for shipping metho…

defc52b

…d selection on cart form

Actually remove js listener for postal code input

cce4ae3

Removed drupal-based listener and added js listener back in.

0194217

We can't seem to add ajax listeners after the fact using #after_build, so the js listener is going back in as our best bet - we might be able to avoid clicking the button though and instead trigger an event durpal- side.

Remove #after_build on shipping profile form

3d8b0c7

travis-bradbury suggested changes Aug 9, 2018

View reviewed changes

Gabriel Simmer added 2 commits August 9, 2018 15:18

Added timeout to recalculation button press and fix up some coding st…

8d38d3a

…andards issues

Changed how the calculate shipping function is triggered.

bc5d265

Instead of a blue or change of the postal code, we add a listener to every input element instead, and wait until all the required fields have been filled.

Comments

Conversation

gmemstr commented Jul 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmemstr commented Aug 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gmemstr commented Jul 27, 2018 •

edited

Loading