Admin cart page revamp #1710

jhawthorn · 2017-02-09T00:58:21Z

This builds on top of #1709.

New Order

We display the "new item" row immediately
"Add item" is disabled (we are already adding a row)
The "cancel" button is removed

Validation failed

After clicking save
blank fields are highlighted in red

After saving one item

"Add item" button is enabled

Adding second item

"Add item" button is disabled
Cancel button is available (clicking it removes the row and re-enables the "Add item button")

tvdeyen · 2017-02-10T07:56:27Z

core/lib/spree/testing_support/capybara_ext.rb

+        within(#{table_column.inspect}) do
+          fill_in #{selector.inspect}, with: #{quantity.inspect}
+        end
+    WARN


Ah, you already did :)

Mandily · 2017-02-10T22:34:55Z

👍 Looks good to me!

vfonic · 2017-02-17T06:51:51Z

I've never seen the UI in which "Add Item" is disabled. From my experience, the button/link is enabled and, when the user clicks on it, it adds a new empty row (or if we don't want that, we can focus on the above empty row). If the user didn't want to do this, s/he can remove that row.
To me, this UI looks like:
I fill in the cart item details and then "Add Item" button is enabled, so I click on it to confirm adding this first item. Of course this is wrong, since "Add Item" doesn't confirm my action, but the button on the right does.

PS I've noticed that action buttons/icons in solidus admin are usually outside of the table and table row. I think if the action button is referring to the item in the table row, it should be inside the table itself:

Not sure why it was explicitly made look like it doesn't belong to the table.

PPS I'm trying to bring a new "outside" view. Maybe I'm wrong in some things so please either correct me or ignore me if you want to ship things faster. ;)

Mandily · 2017-02-17T19:00:07Z

I also like the idea of putting the icons in the table to keep relative things grouped together, but it might be better to keep it separate for consistency right now, and then globally change the location of all icons in a separate PR. I realize in suggesting that though, someone would have to volunteer to do that chunk of work. This sounds like a thing that I might be capable of, but I'm not sure when I'll have the time.

I can see how on first glance a user might think that the "add item" button at the bottom of the page would save the new row. I know they're figure it out and learn pretty quickly, but in the interest of a more intuitive UI, it would be nice if we could rename the label to "Add new item"

The note about the disabled button raises a bigger discussion about whether we want to put the user into a stateful interaction (which is how Solidus works now) to keep them focused on their task or make it stateless to give them more flexibility. Again, I'm inclined to say keep it consistent until we have a bigger bandwidth to think through and change things on a more global scale.

jhawthorn · 2017-02-17T20:57:20Z

Not sure why it was explicitly made look like it doesn't belong to the table.

This is the pattern used throughout the admin and is way outside the scope of this PR.

If you have thoughts on how the admin UI should be please file a new issue or come discuss it in our #gui slack channel

vfonic · 2017-02-18T10:18:43Z

@jhawthorn

I was talking generally:

PS I've noticed that action buttons/icons in solidus admin are usually outside of the table...

I'll start the discussion in #gui

tvdeyen

Great. Some thoughts about backbone model organizing as discussed in #1715

tvdeyen · 2017-02-28T19:13:30Z

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

+	<a class="edit-line-item fa fa-edit no-text with-tip" href="#" title="{{ t "actions.edit" }}"></a>
+	<a class="delete-line-item fa fa-trash no-text with-tip" href="#" title="{{ t "actions.delete" }}"></a>
+{{/if}}
+</td>


The indenting of this file is a little off

Rebased and fixed.

tvdeyen · 2017-02-28T19:18:51Z

backend/app/assets/stylesheets/spree/backend/sections/_orders.scss

-.line-item:not(.editing) {
-  .save-line-item,
-  .cancel-line-item,
-  .line-item-qty-edit {


Can we remove these classes from the html as well?

.save-line-item and .cancel-line-item are still in use and also used in specs (a more dubious use).

I'm not sure they aren't appropriate classes to have.

.line-item-qty-edit and .line-item-qty-show are JS utility classes used in the old JS only AFAIK. Maybe there are others. Not mandatory, for sure. But I like to clean up as we have the chance.

tvdeyen · 2017-02-28T19:27:06Z

backend/app/assets/javascripts/spree/backend/line_items.js.coffee

+$ ->
+  if $("table.line-items").length
+    url = Spree.routes.orders_api + "/" + order_number
+    lineItemModel = Backbone.Model.extend


Can we put this model into its own file and use the Spree.Model namespace discussed in #1715?

Can't be done here. At this point the model needs to be defined where we have access to url. #1715 is a follow up to this where this will be fixed.

tvdeyen · 2017-02-28T19:30:17Z

backend/app/assets/javascripts/spree/backend/line_items.js.coffee

-      $('.line-items').remove()
-    window.Spree.advanceOrder()
+
+Spree.CartLineItemView = Backbone.View.extend


Maybe we can also introduce a Spree.Views namespace for views as we discussed for models in #1715?

I'd rather address this in the #1715, which is a follow up to this. This PR was intended only to be the UI change, with #1715 being a more complete switch to backbone.

tvdeyen · 2017-02-28T19:37:38Z

backend/app/views/spree/admin/orders/_line_items.html.erb

-  </table>
-<% end %>
+  <tbody>
+  </tbody>


It really helped me today that in the other backbone powered templates we have a comment noting that the content is handled via JS. Maybe we could also add a comment here.

👍 Excellent idea

Not mine ;) d2ec380

This is an unnecessary shorthand which just obfuscates what it does. It also has nothing to do with quantities specifically.

tvdeyen

Thanks for all your late work on making the admin UX better 🙏🏻

jhawthorn · 2017-02-28T22:10:43Z

Can't run test on CircleCI because of AWS issues, but this is passing locally

🔥

jhawthorn added the WIP label Feb 9, 2017

tvdeyen reviewed Feb 10, 2017

View reviewed changes

jhawthorn force-pushed the cart_page_revamp branch from 54cbf5b to 8287bc3 Compare February 10, 2017 21:58

jhawthorn force-pushed the cart_page_revamp branch from 8287bc3 to 751bc58 Compare February 10, 2017 22:55

jhawthorn removed the WIP label Feb 10, 2017

jhawthorn changed the title ~~[WIP] Cart page revamp~~ Cart page revamp Feb 10, 2017

jhawthorn mentioned this pull request Feb 10, 2017

Remove stock location configuration from admin "cart" page #1709

Merged

jhawthorn changed the title ~~Cart page revamp~~ Admin cart page revamp Feb 14, 2017

jhawthorn added changelog:solidus_backend Changes to the solidus_backend gem UI labels Feb 14, 2017

jhawthorn mentioned this pull request Feb 16, 2017

Update admin cart page dynamically #1715

Merged

jhawthorn force-pushed the cart_page_revamp branch from 751bc58 to 38ce62f Compare February 28, 2017 01:04

cbrunsdon approved these changes Feb 28, 2017

View reviewed changes

tvdeyen requested changes Feb 28, 2017

View reviewed changes

jhawthorn and others added 10 commits February 28, 2017 12:13

Deprecate fill_in_quantity

f8fe5c6

This is an unnecessary shorthand which just obfuscates what it does. It also has nothing to do with quantities specifically.

Improve column spacing on cart page

fa75ba2

Render cart page line_items using handlebars

88fe43e

Convert line item edit to Backbone.js

9563773

Use on Spree.CartLineItemView for rendering

b75d78f

Perform save and destroy through backbone model

78bc407

Only run JS for line_items on cart page

4dab19d

Control editing state with variable

2de9d05

Use templtaes to control editing instead of CSS

6e82dc5

Add new line items inline on table

9ed8b34

jhawthorn added 6 commits February 28, 2017 12:13

Remove old line item add JS

12ea658

Show add line item row if order is empty

ee7d883

Add validations to line item creation

d98b617

Add styling for disabled buttons

e4b0af9

Disable button until json is fetched

7c5fd3b

Add note to section rendered by JS

bb6093f

jhawthorn force-pushed the cart_page_revamp branch from 38ce62f to bb6093f Compare February 28, 2017 20:16

tvdeyen approved these changes Feb 28, 2017

View reviewed changes

jhawthorn merged commit 0d9c72b into solidusio:master Feb 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin cart page revamp #1710

Admin cart page revamp #1710

jhawthorn commented Feb 9, 2017 •

edited

Loading

tvdeyen Feb 10, 2017

Mandily commented Feb 10, 2017

vfonic commented Feb 17, 2017

Mandily commented Feb 17, 2017

jhawthorn commented Feb 17, 2017

vfonic commented Feb 18, 2017

tvdeyen left a comment

tvdeyen Feb 28, 2017

jhawthorn Feb 28, 2017

tvdeyen Feb 28, 2017

jhawthorn Feb 28, 2017

tvdeyen Feb 28, 2017

tvdeyen Feb 28, 2017

jhawthorn Feb 28, 2017 •

edited

Loading

tvdeyen Feb 28, 2017

tvdeyen Feb 28, 2017

jhawthorn Feb 28, 2017

tvdeyen Feb 28, 2017

tvdeyen Feb 28, 2017

jhawthorn Feb 28, 2017

tvdeyen Feb 28, 2017

tvdeyen left a comment

jhawthorn commented Feb 28, 2017 •

edited

Loading

Admin cart page revamp #1710

Admin cart page revamp #1710

Conversation

jhawthorn commented Feb 9, 2017 • edited Loading

New Order

Validation failed

After saving one item

Adding second item

Choose a reason for hiding this comment

Mandily commented Feb 10, 2017

vfonic commented Feb 17, 2017

Mandily commented Feb 17, 2017

jhawthorn commented Feb 17, 2017

vfonic commented Feb 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

jhawthorn Feb 28, 2017 • edited Loading

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

Choose a reason for hiding this comment

tvdeyen left a comment

Choose a reason for hiding this comment

jhawthorn commented Feb 28, 2017 • edited Loading

jhawthorn commented Feb 9, 2017 •

edited

Loading

jhawthorn Feb 28, 2017 •

edited

Loading

jhawthorn commented Feb 28, 2017 •

edited

Loading