Move editing of shipment method and tracking into JS views and templates #2225

jhawthorn · 2017-09-18T22:54:24Z

This is the first step towards future work on two separate goals:

Allowing editing shipments fully through ajax without the page being refreshed
Allowing backend-only shipping methods to be used by admins Not able to use backend shipping methods in admin #1711

For both of these, we need the editing of shipping methods to be renderable by JS (it was previously an ERB view). This also does the same to he tracking method, which made sense to do at the same time since they used to share code.

This creates a Shipment backbone model and collection, and fetches it from the order API endpoint when loading the shipments page.

Two new handlebars helpers were needed to allow this: human_model_name which works like model_name.human in ruby (but only supports the singular translation for now), and format_money which gives access to the existing Spree.formatMoney function.

tvdeyen

Thanks. Poked around in the sandbox and it's working well.

Well, besides that we do not let users submit our Backbone views with the enter key. 😿

I described below in the comments how this can be easily achieved. By using the barely known magic form attribute of buttons.

I will send a PR with the changes that are needed to make this work. A huge improvement to UX we should embrace for all our Backbone based forms.

/c @Mandily

tvdeyen · 2017-09-19T04:58:38Z

backend/app/assets/javascripts/spree/backend/collections/shipments.js

+  model: Spree.Models.Shipment,
+
+  url: function () {
+    return Spree.routes.shipments_api;


This doesn’t have to be a function if we use a static property

tvdeyen · 2017-09-19T05:02:56Z

backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js

+    "click .js-edit":   "onEdit",
+    "click .js-save":   "onSave",
+    "click .js-cancel": "onCancel",
+    "change select": "onChange",


The indentation is a little off here.

Just saw, that the onChange function is not defined in this view.

tvdeyen · 2017-09-19T05:29:32Z

backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js

+      patch: true,
+      success: function() {
+        // FIXME: should update page without reloading
+        window.location.reload();


What is the reason that we need a full page reload? Just curious

Changing the shipping method may change the amount of the order, so the sidebar needs updating. This will eventually be done without a reload, but is outside the scope of this PR (which is the first step towards removing it).

This doesn't introduce the reload, it was moved from here

Thanks for clarification

tvdeyen · 2017-09-21T08:17:50Z

backend/app/assets/javascripts/spree/backend/views/order/shipment_tracking.js

+
+  events: {
+    "click .js-edit":   "onEdit",
+    "click .js-save":   "onSave",


It's very inconvenient one can't submit all our Backbone views with Enter key. We should change that.

If we would listen to submit form events instead and wrap the fields into a form (and use that magic submit button trick shown below) we can have both behaviours (enter key and click on the button) with the same event handler.

I will submit a PR with the changes to your PR.

tvdeyen · 2017-09-21T08:31:05Z

backend/app/assets/javascripts/spree/backend/templates/orders/shipment_tracking.hbs

+</th>
+<td colspan="4">
+  {{#if editing}}
+    <input type="text" name="tracking" class="fullwidth" value="{{tracking}}">


If we would wrap this in a form we could use the submit event instead.

<form id="tracking_form">

Notice that we need to have that id to connect the "save" button with this form as it sits outside of the form. But there is this barely known attribute of submit buttons called form.

Very handy for our use case!

tvdeyen · 2017-09-21T08:35:13Z

backend/app/assets/javascripts/spree/backend/templates/orders/shipment_tracking.hbs

+</td>
+<td class="actions">
+  {{#if editing}}
+    <button class="js-save fa fa-check no-text with-tip" data-action="save" title="{{ t "actions.save" }}"></button>


In order to use the same submit event we need to add

<button type="submit" form="tracking_form">

the form attribute is important here.

tvdeyen · 2017-09-21T18:20:45Z

Just as a heads up. Besides the PR to this PR here I am working on updating all Backbone based views to also support submitting via enter key (where it’s appropriate)

jhawthorn · 2017-09-29T19:18:16Z

I believe I've addressed the suggestions here.

The last 4 commits here are poorly named and will be squashed into earlier commits, but I have kept them separate to ease reviews.

tvdeyen

YAY!

Previously, this spec used some error-prone DOM manipulation: within_row(1) worked when there were two shipments only because the first row of the second table was a hidden tr (for edit shipping method) Some specs required specific css classes and were tied to the editing implementation hiding/showing different rows. Both of these issues are improved by matching rows based on their text using within('tr', text: ...) This also fixes an issue where a second shipment was unintentionally created in some specs.

This doesn't handle pluralization yet, we can add that when it becomes necessary.

Hide shipments until data is available

In order to give users the same UX they know from form fields, we need to wrap the tracking input field into a form and use a submit button as save button. Because the submit buttons sits outside of the form and we can't wrap the whole table row in a form (HTML semantics) we need to use the barely known form attribute of submit buttons. Now we can use the form submit event instead of the save button click event and one can submit the form by clicking the save button or by hitting the enter key inside the form. Use form submit event for saving order shipment

jhawthorn added the changelog:solidus_backend Changes to the solidus_backend gem label Sep 18, 2017

jhawthorn force-pushed the admin_backbone_shipment_edit branch from 9b2400e to 05cccac Compare September 18, 2017 23:05

tvdeyen reviewed Sep 21, 2017

View reviewed changes

tvdeyen mentioned this pull request Sep 21, 2017

Order shipping form submit events jhawthorn/solidus#5

Closed

jhawthorn force-pushed the admin_backbone_shipment_edit branch 2 times, most recently from 7cbf529 to d7d0473 Compare September 29, 2017 19:14

jhawthorn force-pushed the admin_backbone_shipment_edit branch from d7d0473 to ab2a300 Compare September 29, 2017 20:06

tvdeyen approved these changes Sep 29, 2017

View reviewed changes

jhawthorn and others added 10 commits October 3, 2017 14:52

Add human_model_name handlebars helper

85a6648

This doesn't handle pluralization yet, we can add that when it becomes necessary.

Add format_money handlebars helper

e7dac5c

Add backbone Shipment model and collection

c2b6000

Add backbone Shipment model and collection

2dfabb6

Fetch order and shipment backbone model

45dd2a6

Hide shipments until data is available

Use backbone model to get numbers

7bd0ee3

Move shipping method to own backbone view

758414c

Move shipment tracking to own backbone view

5497ee0

jhawthorn force-pushed the admin_backbone_shipment_edit branch from ab2a300 to 72c116a Compare October 3, 2017 21:56

jhawthorn merged commit 926ea68 into solidusio:master Oct 3, 2017

jhawthorn deleted the admin_backbone_shipment_edit branch October 3, 2017 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move editing of shipment method and tracking into JS views and templates #2225

Move editing of shipment method and tracking into JS views and templates #2225

jhawthorn commented Sep 18, 2017

tvdeyen left a comment

tvdeyen Sep 19, 2017

tvdeyen Sep 19, 2017

tvdeyen Sep 21, 2017

tvdeyen Sep 19, 2017

jhawthorn Sep 29, 2017

tvdeyen Sep 29, 2017

tvdeyen Sep 21, 2017 •

edited

Loading

Mandily Sep 21, 2017

tvdeyen Sep 21, 2017 •

edited

Loading

tvdeyen Sep 21, 2017

tvdeyen commented Sep 21, 2017

jhawthorn commented Sep 29, 2017

tvdeyen left a comment

Move editing of shipment method and tracking into JS views and templates #2225

Move editing of shipment method and tracking into JS views and templates #2225

Conversation

jhawthorn commented Sep 18, 2017

tvdeyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvdeyen Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvdeyen Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvdeyen commented Sep 21, 2017

jhawthorn commented Sep 29, 2017

tvdeyen left a comment

Choose a reason for hiding this comment

tvdeyen Sep 21, 2017 •

edited

Loading

tvdeyen Sep 21, 2017 •

edited

Loading